diff --git a/src/web/public/panels-ui.js b/src/web/public/panels-ui.js index 81c7c990..0e5fdfec 100644 --- a/src/web/public/panels-ui.js +++ b/src/web/public/panels-ui.js @@ -380,7 +380,15 @@ Object.assign(CodemanApp.prototype, { prev.focus(); return; } - if (this.activeSessionId) this.terminal?.focus?.(); + // ⚠️ `activeSessionId` alone only covers the welcome screen. On a touch device + // with the keyboard down, focus sits on ``, so focusing the terminal here + // would summon the on-screen keyboard — `selectSession()` deliberately skips the + // focus for exactly that reason, and this would override it. The app's own + // predicate already encodes the rule (true on desktop, on touch only while the + // keyboard is open); the optional call keeps the vm test harness working. + if (this.activeSessionId && this._shouldFocusTerminalForTabSwitch?.() !== false) { + this.terminal?.focus?.(); + } }, openCommandPalette() { @@ -404,7 +412,15 @@ Object.assign(CodemanApp.prototype, { closeCommandPalette() { const modal = document.getElementById('commandPaletteModal'); - if (modal) modal.classList.remove('active'); + // ⚠️ Bail out when it was not open. The global Escape handler calls this on + // EVERY Escape (app.js), in the CAPTURE phase, so an unconditional restore + // runs before the focused element's own Escape handler and steals focus into + // the terminal: keys typed after Escape in split Pane B land in Pane A, keys + // typed in any text field land in the terminal, and the inline tab rename's + // Escape fires the input's blur (which commits) before its own handler + // (which cancels), turning a cancel into a rename. + if (!modal?.classList?.contains('active')) return; + modal.classList.remove('active'); this._restoreOverlayFocus('_commandPalettePrevFocus'); }, @@ -674,7 +690,9 @@ Object.assign(CodemanApp.prototype, { closeSessionManager() { const modal = document.getElementById('sessionManagerModal'); - if (modal) modal.classList.remove('active'); + // Same guard as closeCommandPalette — see the note there. + if (!modal?.classList?.contains('active')) return; + modal.classList.remove('active'); this._restoreOverlayFocus('_sessionManagerPrevFocus'); }, diff --git a/test/command-palette-ui.test.ts b/test/command-palette-ui.test.ts index 70e19ced..00048010 100644 --- a/test/command-palette-ui.test.ts +++ b/test/command-palette-ui.test.ts @@ -111,7 +111,7 @@ function loadPaletteHarness(overrides: Record = {}) { app.getSessionName = (session: any) => session.name || session.workingDir?.split('/').pop() || app.getShortId(session.id); - return { app, elements, listeners }; + return { app, elements, listeners, makeClassList }; } describe('Command-K session palette', () => { @@ -492,7 +492,7 @@ describe('overlay focus restoration (Escape must not strand the keyboard)', () = const priorElement = { focus: vi.fn(), isConnected: true, tagName: 'TEXTAREA' }; const body = { tagName: 'BODY' }; let active: any = priorElement; - const { app, elements } = loadPaletteHarness({ + const { app, elements, makeClassList } = loadPaletteHarness({ document: { getElementById: (id: string) => (globalThis as any).__els?.[id] ?? null, get activeElement() { @@ -509,7 +509,7 @@ describe('overlay focus restoration (Escape must not strand the keyboard)', () = elements.commandPaletteSearch.focus = vi.fn(() => { active = elements.commandPaletteSearch; }); - return { app, elements, priorElement, terminalTextarea, body, setActive: (v: any) => (active = v) }; + return { app, elements, priorElement, terminalTextarea, body, makeClassList, setActive: (v: any) => (active = v) }; } it('returns focus to whatever had it when the command palette closes', () => { @@ -546,9 +546,33 @@ describe('overlay focus restoration (Escape must not strand the keyboard)', () = expect(terminalTextarea.focus).toHaveBeenCalledTimes(1); }); + it('leaves focus alone when neither overlay was open — the path every Escape takes', () => { + // app.js's global Escape handler calls both close methods on EVERY Escape, + // in the capture phase. Nothing was saved, so an unguarded restore would fall + // through to the terminal and steal focus from split Pane B, from any text + // field, and turn the inline rename's Escape into a commit. + const { app, elements, terminalTextarea, priorElement, makeClassList } = focusHarness(); + elements.sessionManagerModal = { classList: makeClassList(), addEventListener: vi.fn() }; + app.closeCommandPalette(); + app.closeSessionManager(); + expect(terminalTextarea.focus).not.toHaveBeenCalled(); + expect(priorElement.focus).not.toHaveBeenCalled(); + }); + + it('does not focus the terminal on touch while the keyboard is down', () => { + const { app, terminalTextarea, body, setActive } = focusHarness(); + app._shouldFocusTerminalForTabSwitch = () => false; + setActive(body); + app.openCommandPalette(); + app.closeCommandPalette(); + expect(terminalTextarea.focus).not.toHaveBeenCalled(); + }); + it('restores focus on the session manager too, not just the palette', async () => { - const { app, elements, priorElement } = focusHarness(); - elements.sessionManagerModal = { classList: { add: vi.fn(), remove: vi.fn() }, addEventListener: vi.fn() }; + const { app, elements, priorElement, makeClassList } = focusHarness(); + // A real classList: the close guard reads `contains('active')`, and a stub + // without it reports "not open" and skips the restore this test is about. + elements.sessionManagerModal = { classList: makeClassList(), addEventListener: vi.fn() }; elements.sessionManagerSearch = { value: '', focus: vi.fn(), addEventListener: vi.fn() }; elements.sessionManagerList = { replaceChildren: vi.fn(), appendChild: vi.fn() }; app._loadSessionManagerList = vi.fn();