fix(ui): stop Escape from stranding the keyboard after closing an overlay

Both the Command Palette and the Session Manager call `search.focus()` on
open, and both closed by removing the `active` class and nothing else. Hiding
a focused input does not hand focus back to anyone — the browser drops it on
`<body>` — so after Escape closed the overlay every keystroke went nowhere and
the user had to click the terminal before they could type again.

Measured in headless chromium against a real shell session, one overlay at a
time:

  overlay            activeElement after Esc   can type afterwards
  App Settings       XTERM                     yes
  Session Options    XTERM                     yes
  Token Stats        XTERM                     yes
  Monitor Panel      XTERM                     yes
  Session Manager    BODY                      no    <- fixed here
  Command Palette    BODY                      no    <- fixed here

The four that worked did so because they use `FocusTrap`, whose `deactivate()`
restores focus to whatever held it before. These two never got one. Every close
path has the same hole — Escape, the close method, picking an item — so the
restore lives in the close functions rather than in the global Escape chain.

Deliberately only the save/restore half of `FocusTrap`, not the whole thing:
`FocusTrap.activate()` moves focus to the first focusable element, which in
neither overlay is the search box, so adopting it wholesale would trade "type a
filter the moment it opens" for "focus survives the close" — and the former is
the reason Cmd+K exists. The terminal fallback is gated on there being an
active session: an overlay opened from the welcome screen has no terminal to
return to, and focusing one on a phone summons the on-screen keyboard over a
screen with no input on it.

The five new cases were checked against the unfixed code first: four of them
fail without this change.
This commit is contained in:
d fei
2026-09-29 18:02:05 -07:00
parent 848ab48b0a
commit af4cc45cfd
2 changed files with 128 additions and 0 deletions
+50
View File
@@ -339,6 +339,50 @@ Object.assign(CodemanApp.prototype, {
return true;
},
/**
* Save what had focus before a search-first overlay takes it.
*
* The Command Palette and the Session Manager both call `search.focus()` on
* open, and both used to close by removing the `active` class and nothing
* else. Hiding the focused input does not hand focus back to anyone — the
* browser drops it on `<body>` — so after Escape closed the overlay every
* keystroke went nowhere and the user had to click the terminal to type
* again (measured: `document.activeElement` is BODY afterwards and the
* terminal emits no onData at all). Every close path has the same hole, so
* the restore lives in the close functions, not in the global Escape chain.
*
* ⚠️ Deliberately only the save/restore half of {@link FocusTrap}, not the
* whole thing. `FocusTrap.activate()` moves focus to the first focusable
* element, which in both of these overlays is not the search box — adopting
* it wholesale would fix the focus loss by breaking the thing Cmd+K exists
* for, typing a filter the moment it opens.
*/
_rememberOverlayFocus(key) {
this[key] = (typeof document !== 'undefined' && document.activeElement) || null;
},
/**
* Hand focus back to whatever {@link _rememberOverlayFocus} saved.
*
* ⚠️ The terminal fallback is gated on there being an active session: an
* overlay opened from the welcome screen has no terminal to return to, and
* focusing one on a phone summons the on-screen keyboard over a screen that
* has no input on it.
*/
_restoreOverlayFocus(key) {
const prev = this[key];
this[key] = null;
const body = typeof document !== 'undefined' ? document.body : null;
// `isConnected === false` means the element was removed while the overlay
// was open (a re-render of the tab strip, say); anything else — including
// a stub with no such property — is treated as still focusable.
if (prev && prev !== body && prev.isConnected !== false && typeof prev.focus === 'function') {
prev.focus();
return;
}
if (this.activeSessionId) this.terminal?.focus?.();
},
openCommandPalette() {
const modal = document.getElementById('commandPaletteModal');
const search = document.getElementById('commandPaletteSearch');
@@ -351,6 +395,9 @@ Object.assign(CodemanApp.prototype, {
this._wireCommandPalette();
this.renderCommandPalette();
// BEFORE the steal, not after: `search.focus()` below is what loses the
// caller's focus, so the read has to happen while it is still there.
this._rememberOverlayFocus('_commandPalettePrevFocus');
search.focus();
search.select?.();
},
@@ -358,6 +405,7 @@ Object.assign(CodemanApp.prototype, {
closeCommandPalette() {
const modal = document.getElementById('commandPaletteModal');
if (modal) modal.classList.remove('active');
this._restoreOverlayFocus('_commandPalettePrevFocus');
},
_wireCommandPalette() {
@@ -618,6 +666,7 @@ Object.assign(CodemanApp.prototype, {
});
}
search.value = '';
this._rememberOverlayFocus('_sessionManagerPrevFocus');
search.focus();
}
await this._loadSessionManagerList('');
@@ -626,6 +675,7 @@ Object.assign(CodemanApp.prototype, {
closeSessionManager() {
const modal = document.getElementById('sessionManagerModal');
if (modal) modal.classList.remove('active');
this._restoreOverlayFocus('_sessionManagerPrevFocus');
},
/** Replace the Session Manager list body with a single status line. */
+78
View File
@@ -480,6 +480,84 @@ describe('Session Manager unified list', () => {
});
});
describe('overlay focus restoration (Escape must not strand the keyboard)', () => {
/**
* Both overlays focus their search box on open. Closing them used to leave
* focus on <body>, so after Escape every keystroke went nowhere until the
* user clicked the terminal — measured in a real browser against a shell
* session: activeElement BODY, zero onData for anything typed afterwards.
*/
function focusHarness() {
const terminalTextarea = { focus: vi.fn(), isConnected: true };
const priorElement = { focus: vi.fn(), isConnected: true, tagName: 'TEXTAREA' };
const body = { tagName: 'BODY' };
let active: any = priorElement;
const { app, elements } = loadPaletteHarness({
document: {
getElementById: (id: string) => (globalThis as any).__els?.[id] ?? null,
get activeElement() {
return active;
},
body,
},
});
(globalThis as any).__els = elements;
app.terminal = { focus: terminalTextarea.focus };
app.activeSessionId = 'sess-beta';
// The overlay's own focus() is what moves focus in a real browser; the
// fake document needs the same transition or the test proves nothing.
elements.commandPaletteSearch.focus = vi.fn(() => {
active = elements.commandPaletteSearch;
});
return { app, elements, priorElement, terminalTextarea, body, setActive: (v: any) => (active = v) };
}
it('returns focus to whatever had it when the command palette closes', () => {
const { app, priorElement } = focusHarness();
app.openCommandPalette();
expect(priorElement.focus).not.toHaveBeenCalled();
app.closeCommandPalette();
expect(priorElement.focus).toHaveBeenCalledTimes(1);
});
it('falls back to the terminal when the prior element is gone, but only with a live session', () => {
const { app, priorElement, terminalTextarea } = focusHarness();
app.openCommandPalette();
priorElement.isConnected = false;
app.closeCommandPalette();
expect(priorElement.focus).not.toHaveBeenCalled();
expect(terminalTextarea.focus).toHaveBeenCalledTimes(1);
});
it('never focuses the terminal from the welcome screen (a phone would pop the keyboard)', () => {
const { app, priorElement, terminalTextarea } = focusHarness();
app.activeSessionId = null;
app.openCommandPalette();
priorElement.isConnected = false;
app.closeCommandPalette();
expect(terminalTextarea.focus).not.toHaveBeenCalled();
});
it('does not restore focus to <body>, which is the bug itself', () => {
const { app, terminalTextarea, body, setActive } = focusHarness();
setActive(body);
app.openCommandPalette();
app.closeCommandPalette();
expect(terminalTextarea.focus).toHaveBeenCalledTimes(1);
});
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() };
elements.sessionManagerSearch = { value: '', focus: vi.fn(), addEventListener: vi.fn() };
elements.sessionManagerList = { replaceChildren: vi.fn(), appendChild: vi.fn() };
app._loadSessionManagerList = vi.fn();
await app.openSessionManager();
app.closeSessionManager();
expect(priorElement.focus).toHaveBeenCalledTimes(1);
});
});
describe('panel close helpers', () => {
it('closes panels when the mobile header helper is unavailable', () => {
const CodemanApp = function CodemanApp(this: any) {};