From 84e4a0bf00cbe4dc6ba5102f00f0d7d0e90a2bfa Mon Sep 17 00:00:00 2001 From: Edwin Date: Mon, 3 Aug 2026 21:49:12 -0700 Subject: [PATCH] webui: restore focus when dismissing modal sheets Closing rename/settings/close-session/new-session only hid the sheet, so keyboard focus fell to and the next keystroke went nowhere. Record the invoker on open, restore it on close, and fall back to the active surface (composer preferred on touch) when the invoker is gone. Also land focus after successful session create and deliberate view-mode toggles. Spec 0189; regression coverage in web_smoke. Fixes #1074. --- crates/daemon/assets/index.html | 125 ++++++++++++++++++++-- crates/e2e/tests/web_smoke.rs | 122 +++++++++++++++++++++ specs/0189-webui-overlay-focus-restore.md | 76 +++++++++++++ 3 files changed, 313 insertions(+), 10 deletions(-) create mode 100644 specs/0189-webui-overlay-focus-restore.md diff --git a/crates/daemon/assets/index.html b/crates/daemon/assets/index.html index 6b78bb99..a8d866c5 100644 --- a/crates/daemon/assets/index.html +++ b/crates/daemon/assets/index.html @@ -8402,13 +8402,23 @@

// toggle. Switching between terminal sessions never did that and worked // fine, so match it — the cached xterm is revealed and the child repaints // on the geometry claim. + const focusTerm = shouldFocusTerminalAfterSessionSwitch(); await enterTerminalMode(id, { - focusTerminal: shouldFocusTerminalAfterSessionSwitch(), + focusTerminal: focusTerm, }); + // A deliberate view toggle must leave a typing destination (issue #1074). + // On touch with the soft keyboard hidden we skip xterm focus, so land on + // the mobile composer instead of leaving the caret on . + if (!focusTerm) { + focusActiveSurface({ preferComposerOnTouch: true }); + } } else { rememberActiveTerminalState(); enterChatMode(); await loadTranscript(id); + // Chat has no enter-time focus path of its own; put the caret in the + // composer so the user can type immediately after the toggle. + focusActiveSurface({ preferComposerOnTouch: true }); } setComposerEnabled(!!(state.ws && state.ws.readyState === 1 && state.currentId)); reportView(); @@ -17719,6 +17729,8 @@

const newSessionCreateBtn = $("newSessionCreate"); async function openNewSessionDialog() { + // Capture before anything in this dialog takes the caret (issue #1074). + const invoker = captureOverlayInvoker(); // Populate the harness dropdown from `harness.list`. Filtered to // available harnesses so the user can't pick one that the daemon // would reject. @@ -17729,6 +17741,7 @@

} catch (e) { newSessionErrorEl.textContent = `harness.list failed: ${e.message}`; newSessionErrorEl.hidden = false; + newSessionSheetEl._overlayInvoker = invoker; newSessionSheetEl.hidden = false; return; } @@ -17747,16 +17760,21 @@

const cur = state.sessions.find((s) => s.id === state.currentId); newSessionCwdEl.value = (cur && cur.cwd) || ""; newSessionPromptEl.value = ""; + newSessionSheetEl._overlayInvoker = invoker; newSessionSheetEl.hidden = false; // Defer focus so the slide-up animation doesn't fight the // keyboard-summon path. setTimeout(() => newSessionHarnessEl.focus(), 200); } -function closeNewSessionDialog() { +function closeNewSessionDialog(opts = {}) { + const restore = opts.restore !== false; + const invoker = newSessionSheetEl._overlayInvoker; + newSessionSheetEl._overlayInvoker = null; newSessionSheetEl.hidden = true; newSessionErrorEl.hidden = true; newSessionCreateBtn.disabled = false; + if (restore) restoreOverlayFocus(invoker); } function currentTerminalSize() { @@ -17805,12 +17823,17 @@

} try { const res = await rpc("session.create", params); - closeNewSessionDialog(); + // Don't bounce focus back to the + button — the new session is the + // surface the user is about to type into (issue #1074). + closeNewSessionDialog({ restore: false }); // session/state notifications will repopulate the list; also // auto-select the new session so the user sees it immediately. if (res && res.session_id) { await refreshSessions(); - selectSession(res.session_id); + await selectSession(res.session_id); + focusActiveSurface({ preferComposerOnTouch: true }); + } else { + restoreOverlayFocus(newSessionBtn); } } catch (e) { newSessionCreateBtn.disabled = false; @@ -17840,6 +17863,8 @@

function openRenameDialog(sessionId) { const s = state.sessions.find((x) => x.id === sessionId); if (!s) return; + // Capture before the deferred input focus takes the caret (issue #1074). + renameSheetEl._overlayInvoker = captureOverlayInvoker(); renameFormEl.dataset.sessionId = sessionId; renameTitleEl.value = s.title || ""; renameErrorEl.hidden = true; @@ -17852,10 +17877,13 @@

} function closeRenameDialog() { + const invoker = renameSheetEl._overlayInvoker; + renameSheetEl._overlayInvoker = null; renameSheetEl.hidden = true; renameErrorEl.hidden = true; renameSaveBtn.disabled = false; delete renameFormEl.dataset.sessionId; + restoreOverlayFocus(invoker); } async function submitRename(ev) { @@ -17914,15 +17942,24 @@

function openCloseSessionDialog(sessionId) { const s = state.sessions.find((x) => x.id === sessionId); if (!s) return; + closeSessionSheetEl._overlayInvoker = captureOverlayInvoker(); closeSessionSheetEl.dataset.sessionId = sessionId; const label = sessionDisplayTitle(s) || sessionId.slice(0, 8); closeSessionLabelEl.textContent = `"${label}"`; closeSessionSheetEl.hidden = false; + // Land caret inside the sheet so Escape / Tab stay in the dialog, and so + // close always has a focus transition to restore from (issue #1074). + setTimeout(() => { + try { closeSessionCancelBtn.focus(); } catch (_) {} + }, 0); } function closeCloseSessionDialog() { + const invoker = closeSessionSheetEl._overlayInvoker; + closeSessionSheetEl._overlayInvoker = null; closeSessionSheetEl.hidden = true; delete closeSessionSheetEl.dataset.sessionId; + restoreOverlayFocus(invoker); } closeSessionCancelBtn.addEventListener("click", closeCloseSessionDialog); @@ -18047,11 +18084,21 @@

const settingsSheetEl = $("settingsSheet"); function openSettingsSheet() { + settingsSheetEl._overlayInvoker = captureOverlayInvoker(); settingsSheetEl.hidden = false; + setTimeout(() => { + const closeBtn = $("settingsClose"); + if (closeBtn) { + try { closeBtn.focus(); } catch (_) {} + } + }, 0); } function closeSettingsSheet() { + const invoker = settingsSheetEl._overlayInvoker; + settingsSheetEl._overlayInvoker = null; settingsSheetEl.hidden = true; + restoreOverlayFocus(invoker); } miniUsageGraphEl.addEventListener("click", openSettingsSheet); @@ -18769,18 +18816,76 @@

} } -/** Put the caret back where typing should land for the current view. */ -function focusActiveSurface() { +/** + * Put the caret back where typing should land for the current view. + * + * `preferComposerOnTouch`: when restoring after an overlay dismiss (issue + * #1074), touch layouts prefer the composer over xterm so the user gets a + * visible focus ring / soft keyboard rather than a silent helper-textarea + * focus that looks like the caret vanished. + */ +function focusActiveSurface(opts = {}) { if (state.mode === "playbook") { - playbookInputEl.focus(); + try { playbookInputEl.focus(); } catch (_) {} return; } if (state.mode === "terminal") { + if (opts.preferComposerOnTouch && isLikelyTouchDevice() && composerEl && !composerEl.hidden) { + try { inputEl.focus({ preventScroll: true }); } catch (_) {} + return; + } const handle = state.terminalById.get(state.currentId); - if (handle) handle.term.focus(); - return; + if (handle && handle.term) { + try { handle.term.focus(); } catch (_) {} + return; + } } - inputEl.focus(); + try { inputEl.focus({ preventScroll: true }); } catch (_) {} +} + +/** + * Overlay focus restore (issue #1074 / spec 0189). + * + * Every dismissible sheet records who had focus when it opened. On close we + * put the caret back on that invoker when it is still focusable in the page; + * otherwise we land on the active surface so the next keystroke is not eaten + * by ``. + */ +function captureOverlayInvoker() { + const active = document.activeElement; + if (!active || active === document.body || active === document.documentElement) { + return null; + } + return active; +} + +function overlayInvokerIsRestorable(el) { + if (!el || !el.isConnected || typeof el.focus !== "function") return false; + if (el.disabled || el.getAttribute("aria-disabled") === "true") return false; + // A node inside a [hidden] ancestor (closed menu, closed sheet) is not a + // useful restore target — the browser would bounce focus to . + if (el.closest("[hidden]")) return false; + try { + const style = window.getComputedStyle(el); + if (style.display === "none" || style.visibility === "hidden") return false; + } catch (_) { + /* computed style is best-effort */ + } + return true; +} + +function restoreOverlayFocus(invoker) { + try { + if (overlayInvokerIsRestorable(invoker)) { + invoker.focus({ preventScroll: true }); + if (document.activeElement === invoker || invoker.contains(document.activeElement)) { + return; + } + } + } catch (_) { + /* fall through to the active surface */ + } + focusActiveSurface({ preferComposerOnTouch: true }); } /** Scroll whichever surface the current view is showing. */ diff --git a/crates/e2e/tests/web_smoke.rs b/crates/e2e/tests/web_smoke.rs index 359e4ae8..40167094 100644 --- a/crates/e2e/tests/web_smoke.rs +++ b/crates/e2e/tests/web_smoke.rs @@ -1858,6 +1858,128 @@ async fn web_client_loads_and_websocket_connects() { "clicking an already-focused terminal split must still claim PTY geometry" ); + // Overlay dismiss must restore the invoker (or the active surface) so the + // next keystroke never lands on (issue #1074 / spec 0189). + let overlay_focus: serde_json::Value = page + .evaluate( + r#" + (async () => { + const sleep = (ms) => new Promise((r) => setTimeout(r, ms)); + const activeTag = () => { + const a = document.activeElement; + if (!a || a === document.body) return 'BODY'; + return a.id || a.tagName; + }; + + // Ensure a current session so rename/close-session open paths work. + if (!state.currentId) { + state.sessions = state.sessions || []; + if (!state.sessions.some((s) => s.id === 's-focus-overlay')) { + state.sessions.push({ + id: 's-focus-overlay', + title: 'Focus Overlay', + has_pty: true, + mode: 'interactive', + }); + } + state.currentId = 's-focus-overlay'; + } + + // --- Rename: open from a live invoker, cancel → invoker --- + // (titleRenameBtn may be [hidden] depending on session actions, + // so use a dedicated button we control.) + const renameInvoker = document.createElement('button'); + renameInvoker.id = 'renameInvokerProbe'; + renameInvoker.type = 'button'; + renameInvoker.textContent = 'rename-probe'; + document.body.appendChild(renameInvoker); + renameInvoker.focus(); + openRenameDialog(state.currentId); + await sleep(250); + const renameFocusedWhileOpen = document.activeElement === renameTitleEl; + closeRenameDialog(); + await sleep(0); + const afterRenameCancel = activeTag(); + const afterRenameIsBody = document.activeElement === document.body; + renameInvoker.remove(); + + // --- Settings: open from badge, close → invoker --- + // Canvas is focusable via tabindex set by the badge paint path. + miniUsageGraphEl.setAttribute('tabindex', '0'); + miniUsageGraphEl.focus(); + openSettingsSheet(); + await sleep(20); + closeSettingsSheet(); + await sleep(0); + const afterSettings = activeTag(); + const afterSettingsIsBody = document.activeElement === document.body; + + // --- Close-session: cancel → not body --- + openCloseSessionDialog(state.currentId); + await sleep(20); + closeCloseSessionDialog(); + await sleep(0); + const afterCloseSessionIsBody = document.activeElement === document.body; + + // --- restoreOverlayFocus falls back when invoker is gone --- + const ghost = document.createElement('button'); + ghost.id = 'ghostInvoker'; + document.body.appendChild(ghost); + ghost.focus(); + openRenameDialog(state.currentId); + await sleep(250); + ghost.remove(); + closeRenameDialog(); + await sleep(0); + const afterGhostGoneIsBody = document.activeElement === document.body; + const afterGhostGoneTag = activeTag(); + + return { + renameFocusedWhileOpen, + afterRenameCancel, + afterRenameIsBody, + afterSettings, + afterSettingsIsBody, + afterCloseSessionIsBody, + afterGhostGoneIsBody, + afterGhostGoneTag, + }; + })() + "#, + ) + .await + .expect("evaluate overlay focus restore") + .into_value::() + .expect("json object"); + assert_eq!( + overlay_focus["renameFocusedWhileOpen"], true, + "rename sheet should move focus into its input: {overlay_focus:?}" + ); + assert_eq!( + overlay_focus["afterRenameIsBody"], false, + "closing rename must not leave focus on : {overlay_focus:?}" + ); + assert_eq!( + overlay_focus["afterRenameCancel"], "renameInvokerProbe", + "closing rename should restore the invoker button: {overlay_focus:?}" + ); + assert_eq!( + overlay_focus["afterSettingsIsBody"], false, + "closing settings must not leave focus on : {overlay_focus:?}" + ); + assert_eq!( + overlay_focus["afterSettings"], "miniUsageGraph", + "closing settings should restore the usage badge: {overlay_focus:?}" + ); + assert_eq!( + overlay_focus["afterCloseSessionIsBody"], false, + "closing close-session must not leave focus on : {overlay_focus:?}" + ); + assert_eq!( + overlay_focus["afterGhostGoneIsBody"], false, + "when the invoker is gone, focus must fall back to a surface, not : {overlay_focus:?}" + ); + // xterm emits child-requested mouse reports and automatic terminal // protocol replies through the same onData path as keystrokes. Plain // pointer motion and protocol replies must stay passive; a drag is diff --git a/specs/0189-webui-overlay-focus-restore.md b/specs/0189-webui-overlay-focus-restore.md new file mode 100644 index 00000000..fedfbb2e --- /dev/null +++ b/specs/0189-webui-overlay-focus-restore.md @@ -0,0 +1,76 @@ +# 0189-webui-overlay-focus-restore + +Status: accepted +Date: 2026-08-03 +Area: webui +Scope: Dismissible modal sheets return keyboard focus to their invoker (or the active surface) so keystrokes never land on ``. + +## Decision + +Every dismissible overlay in the web UI (rename, settings, close-session, +new-session, and future sheets of the same shape) records the element that +held keyboard focus when it opened and restores that focus when the sheet +closes — whether closed by Escape, Cancel, backdrop click, or a successful +primary action. + +If the invoker is gone or no longer focusable (row re-rendered, menu item +inside a now-hidden menu, deleted session), the caret falls back to the +active typing surface for the current view (terminal, composer, or playbook +editor). It never ends on ``. + +On touch layouts, the fallback prefers the composer when it is visible over +silently focusing the terminal helper textarea, so the user gets a visible +focus ring / soft keyboard rather than a caret that appears to have vanished. + +A successful "create session" path is an exception to invoker restore: focus +moves to the newly selected session's active surface instead of bouncing back +to the new-session button. + +Deliberate view-mode toggles (chat / terminal / playbook) also leave a typing +destination — chat lands in the composer; playbook lands in the editor; +terminal follows the existing touch-keyboard policy and falls back to the +mobile composer when xterm focus is skipped. + +## Reason + +Opening a sheet correctly moves focus into the dialog, but closing used to +only flip `hidden`. The focused control disappears with the sheet, and the +browser parks the caret on ``. The next keystroke then goes nowhere — +not the terminal, not the composer, not the session list — which reads as the +app being broken. + +The invoker is the right restore target when the user opened the sheet from a +button they still have; the active surface is the right fallback when the +invoker no longer exists. Touch devices need the composer preference because +focusing xterm's helper textarea without a visible caret is worse than a +visible soft-keyboard entry point. + +## Consequences + +- New dismissible sheets must capture the invoker on open and call the shared + restore helper on every close path (including Escape via the overlay stack). +- Approval sheets remain out of scope: they are answered decisions, not + dismissible overlays, and must not be Escape-dismissed. +- Restoring focus must not scroll the page (`preventScroll` where available). +- Ambient repaints still follow + [0184-a-repaint-never-takes-the-caret](0184-a-repaint-never-takes-the-caret.md); + this decision covers deliberate overlay lifecycle, not fleet-driven rebuilds. + +## Non-Goals + +- Building a general focus trap / roving tabindex system for every dialog. +- Changing which chords or keys dismiss overlays. +- Forcing the soft keyboard open on every session list selection (that remains + governed by the touch session-switch policy). + +## Examples + +- Open Rename from the title-bar button, press Escape → focus returns to the + rename button. Type → the keystroke reaches the chord machine / active + surface as before the sheet, not ``. +- Open Settings from the usage badge, click the backdrop → focus returns to + the badge. +- Open Close-session from a list row that is then deleted by another client → + focus lands on the terminal or composer for the current session. +- Create a session from the new-session sheet → focus lands on the new + session's surface so the first typed character is input, not lost.