From d56b90b7c6c4673ddcb37270503f437d9ab4a061 Mon Sep 17 00:00:00 2001 From: JD Date: Wed, 21 Oct 2026 21:09:44 -0400 Subject: [PATCH] fix(mobile): keep the keyboard up when an accessory key is pressed An accessory key that acts on the terminal (arrows, Tab, Esc, Ctrl, scroll, clear input) took focus on press, so every tap blurred the terminal and the click handler had to focus it again: a blur/focus pair per tap that started a keyboard dismissal and shifted the toolbar each time. Such a key now cancels the mousedown default instead, the same pattern as the Attach input, so the terminal never loses focus on the press; the click-time refocus stays as the fallback. A first tap on clear/compact only arms the confirm and is treated the same way. --- docs/architecture-invariants.md | 2 + src/web/CLAUDE.md | 2 +- src/web/public/keyboard-accessory.js | 21 +++- test/accessory-keeps-terminal-focus.test.ts | 105 ++++++++++++++++++++ 4 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 test/accessory-keeps-terminal-focus.test.ts diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index a50539c8..28048d92 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -1068,6 +1068,8 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u ⚠️ **A scroll ends in `touchend` too** — dismissing there closes the keyboard and drops the composer mid-read, so travel is tracked from `touchstart` and multi-touch is never a tap. Both classifiers MUST share one threshold: `initTerminal`'s `TAP_THRESHOLD` reads `MOBILE_KEYBOARD_DISMISS_TAP_SLOP`, since a gesture the terminal calls a scroll and the dismiss handler calls a tap is exactly that bug. +⚠️ **An accessory key that keeps the keyboard up must not take focus on the press.** The keys that act on the terminal and leave the keyboard open (`ACCESSORY_REFOCUS_ACTIONS` in keyboard-accessory.js: scroll, arrows, Tab, Shift+Tab, Ctrl, Esc, clear input and the rest) cancel the `mousedown` default, the same pattern as the Attach input, so the hidden textarea keeps focus through the press; the `terminal.focus()` in the click handler stays as the fallback. Letting the tap blur the terminal and refocusing it afterwards made every such key start a keyboard dismissal, a blur/focus pair per tap that shifted the toolbar each time. A first tap on clear/compact only arms the confirm and is treated the same; the confirming tap, and keys that open a dialog or dismiss the keyboard on purpose, keep the default. Pinned against the real module by `test/accessory-keeps-terminal-focus.test.ts`. + ⚠️ **The gate excludes `test/mobile/**`, so CI cannot see the only test covering (1)** — run `npm run test:mobile -- test/mobile/keyboard.test.ts` by hand and diff the FAIL list against master. (Not `npm test --`: the gate's config excludes that path, so a file filter pointing into it matches nothing and exits green having run zero tests.) That blind spot is why merging the two PRs, which conflicted semantically but not textually, produced a red suite with two green CI checks. ### SSE staleness watchdog diff --git a/src/web/CLAUDE.md b/src/web/CLAUDE.md index 30ef4952..328d0b73 100644 --- a/src/web/CLAUDE.md +++ b/src/web/CLAUDE.md @@ -76,7 +76,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **WebSocket output-gap reconcile** (`_wsOutputGapSession`, app.js): output frames carry no sequence number, so an unintentional WS close while SSE stays up marks the session and the next open reconciles. ⚠️ The marker is cleared only once a repaint actually happened (`_markTerminalBufferReconciled()`, never in a `finally`). → [architecture-invariants#websocket-output-gap-reconcile](../../docs/architecture-invariants.md#websocket-output-gap-reconcile) -**Dismissing the on-screen keyboard** (`terminal-ui.js`): two gestures blur the terminal's hidden textarea. (1) `_installMobileKeyboardDismiss()`, a document `touchend` that must never fire inside `#terminalContainer` or on a control (`MOBILE_KEYBOARD_DISMISS_EXEMPT_SELECTOR`, via `closest()`; roving-tabindex items sit at `tabindex=-1`, so the grouped rail's are listed as `[role="treeitem"]`). (2) In `_handleMobileTerminalTap`, a second tap on inert `content` blurs; the prompt row keeps focus-then-position. ⚠️ A scroll also ends in `touchend`: both classifiers must share one threshold (`TAP_THRESHOLD` reads `MOBILE_KEYBOARD_DISMISS_TAP_SLOP`), and multi-touch is never a tap. ⚠️ CI cannot see the only test for (1): run `npm run test:mobile -- test/mobile/keyboard.test.ts` by hand and diff the FAIL list against master. → [architecture-invariants#dismissing-the-on-screen-keyboard](../../docs/architecture-invariants.md#dismissing-the-on-screen-keyboard) +**Dismissing the on-screen keyboard** (`terminal-ui.js`): two gestures blur the terminal's hidden textarea. (1) `_installMobileKeyboardDismiss()`, a document `touchend` that must never fire inside `#terminalContainer` or on a control (`MOBILE_KEYBOARD_DISMISS_EXEMPT_SELECTOR`, via `closest()`; roving-tabindex items sit at `tabindex=-1`, so the grouped rail's are listed as `[role="treeitem"]`). (2) In `_handleMobileTerminalTap`, a second tap on inert `content` blurs; the prompt row keeps focus-then-position. ⚠️ A scroll also ends in `touchend`: both classifiers must share one threshold (`TAP_THRESHOLD` reads `MOBILE_KEYBOARD_DISMISS_TAP_SLOP`), and multi-touch is never a tap. ⚠️ An accessory key that keeps the keyboard up (`ACCESSORY_REFOCUS_ACTIONS`, keyboard-accessory.js) cancels its `mousedown` default, so the press does not blur the terminal; the click-time `terminal.focus()` stays as the fallback. Letting the tap blur and refocusing afterwards made every such key start a dismissal, a blur/focus pair per tap. A first tap on clear/compact only arms the confirm and is treated the same (`test/accessory-keeps-terminal-focus.test.ts`). ⚠️ CI cannot see the only test for (1): run `npm run test:mobile -- test/mobile/keyboard.test.ts` by hand and diff the FAIL list against master. → [architecture-invariants#dismissing-the-on-screen-keyboard](../../docs/architecture-invariants.md#dismissing-the-on-screen-keyboard) **Phone toolbar: Enter replaces Shell** (post-1.8.0): inside `@media (max-width: 599px)` `btn-shell` is `display:none` and `btn-enter` takes its slot (`order: 4`); starting a shell moved into the Run dropdown (`Terminal / Shell` → `setRunMode('shell')` → `run()` → `runShell()`, button label "Run SH"). `runMode` is `z.string().max(20)` server-side, so new modes need no schema change. Desktop and tablet keep the green Run Shell button unchanged. diff --git a/src/web/public/keyboard-accessory.js b/src/web/public/keyboard-accessory.js index d76aa9c1..33391135 100644 --- a/src/web/public/keyboard-accessory.js +++ b/src/web/public/keyboard-accessory.js @@ -647,6 +647,9 @@ const COMPOSER_INPUT_FRAME_LIMIT = 64 * 1024; const COMPOSER_PASTE_START = '\x1b[200~'; const COMPOSER_PASTE_END = '\x1b[201~'; +// Accessory keys that act on the terminal and leave the keyboard up. +const ACCESSORY_REFOCUS_ACTIONS = new Set(['scroll-up', 'scroll-down', 'arrow-left', 'arrow-right', 'tab', 'shift-tab', 'shift-left', 'shift-right', 'ctrl', 'ctrl-o', 'opt-enter', 'esc', 'effort-max', 'clear-input']); + /** * KeyboardAccessoryBar - Quick action buttons shown above keyboard when typing. */ @@ -802,6 +805,21 @@ const KeyboardAccessoryBar = { this.syncCodexKeys(); this._syncComposerDraftIndicator(); + // A key that keeps the keyboard up must not take focus on press at all. + // Letting the tap blur the terminal and refocusing it in the click handler + // (kept below as the fallback) made every such key start a keyboard + // dismissal: a blur/focus pair per tap. Same pattern as the Attach input + // (image-input.js). A first tap on clear/compact only arms the confirm. + this.element.addEventListener('mousedown', (e) => { + const btn = e.target.closest('.accessory-btn'); + if (!btn) return; + const action = btn.dataset.action; + if (ACCESSORY_REFOCUS_ACTIONS.has(action) || + ((action === 'clear' || action === 'compact') && this._confirmAction !== action)) { + e.preventDefault(); + } + }); + // Add click handlers — preventDefault stops event from reaching terminal this.element.addEventListener('click', (e) => { const btn = e.target.closest('.accessory-btn'); @@ -813,8 +831,7 @@ const KeyboardAccessoryBar = { this.handleAction(action, btn); // Refocus terminal so keyboard stays open (tap blurs terminal → keyboard dismisses → toolbar shifts) - const refocusActions = new Set(['scroll-up', 'scroll-down', 'arrow-left', 'arrow-right', 'tab', 'shift-tab', 'shift-left', 'shift-right', 'ctrl', 'ctrl-o', 'opt-enter', 'esc', 'effort-max', 'clear-input']); - if (refocusActions.has(action) || + if (ACCESSORY_REFOCUS_ACTIONS.has(action) || ((action === 'clear' || action === 'compact') && this._confirmAction)) { if (typeof app !== 'undefined' && app.terminal) { app.terminal.focus(); diff --git a/test/accessory-keeps-terminal-focus.test.ts b/test/accessory-keeps-terminal-focus.test.ts new file mode 100644 index 00000000..a9e979ee --- /dev/null +++ b/test/accessory-keeps-terminal-focus.test.ts @@ -0,0 +1,105 @@ +/** + * @fileoverview Accessory keys that keep the keyboard up must not take focus on + * press. A tap used to blur the terminal's textarea and the click handler then + * refocused it, so every arrow/Tab/Esc tap started a keyboard dismissal. The bar + * now cancels the mousedown default for exactly those keys (and the first, + * arming tap of clear/compact); every other key keeps the old behavior. + * + * Loaded with `vm` against a minimal fake DOM, like the other accessory-bar tests. + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +type Listener = (e: unknown) => void; +type Bar = { + init(): void; + _confirmAction: string | null; + syncReadMyMind(): void; + syncCodexKeys(): void; + _syncComposerDraftIndicator(): void; +}; + +function loadBar() { + const listeners: Record = {}; + const element = { + className: '', + innerHTML: '', + addEventListener: (type: string, fn: Listener) => { + listeners[type] = fn; + }, + }; + const context = vm.createContext({ + console, + document: { createElement: () => element, querySelector: () => null }, + MobileDetection: { isTouchDevice: () => true }, + localStorage: { getItem: () => null, setItem: () => {} }, + }); + vm.runInContext( + `${readFileSync(resolve('src/web/public/keyboard-accessory.js'), 'utf8')}\nglobalThis.__bar = KeyboardAccessoryBar;`, + context + ); + const bar = (context as unknown as { __bar: Bar }).__bar; + bar.syncReadMyMind = () => {}; + bar.syncCodexKeys = () => {}; + bar._syncComposerDraftIndicator = () => {}; + bar.init(); + // `null` is a press on the bar itself (padding, a gap): closest() finds no key. + const press = (action: string | null) => { + const preventDefault = vi.fn(); + listeners.mousedown({ + target: { closest: () => (action === null ? null : { dataset: { action } }) }, + preventDefault, + }); + return preventDefault.mock.calls.length > 0; + }; + return { bar, press }; +} + +describe('accessory keys keep the terminal focused on press', () => { + it('cancels the press default for keys that leave the keyboard up', () => { + const { press } = loadBar(); + for (const action of [ + 'scroll-up', + 'scroll-down', + 'arrow-left', + 'arrow-right', + 'tab', + 'shift-tab', + 'shift-left', + 'shift-right', + 'ctrl', + 'ctrl-o', + 'opt-enter', + 'esc', + 'effort-max', + 'clear-input', + ]) { + expect(press(action), action).toBe(true); + } + }); + + it('leaves keys that move focus or dismiss the keyboard alone', () => { + const { press } = loadBar(); + for (const action of ['compose', 'readmymind', 'pick-path', 'paste', 'dismiss', 'init']) { + expect(press(action), action).toBe(false); + } + }); + + it('cancels only the arming tap of clear/compact, not the confirming one', () => { + const { bar, press } = loadBar(); + bar._confirmAction = null; + expect(press('clear')).toBe(true); + bar._confirmAction = 'clear'; + expect(press('clear')).toBe(false); + expect(press('compact')).toBe(true); + bar._confirmAction = 'compact'; + expect(press('compact')).toBe(false); + }); + + it('ignores a press outside a key', () => { + const { press } = loadBar(); + expect(press(null)).toBe(false); + }); +});