feat(webui): one keyboard shortcut registry with a discoverable ? overlay - #243
Merged
Merged
Conversation
…rlay Closes use-agent-os#137. The console already had real key handling — Enter/Shift+Enter in the composer, Escape's abort/recover chain, Cmd/Ctrl+Shift+O for New chat, arrow navigation in the slash menu and the session switcher — spread across five files as ad-hoc document listeners. Nothing could enumerate them, so nothing documented them, and each new binding re-litigated the same overlay and editable-target guards. Registry. Components declare a ShortcutSpec via useKeyboardShortcut; the provider owns the single document listener, the editable-target guard and the overlay guard. Keys that must stay bound to their own element (composer, slash menu, session switcher — they need the event before the document and have to consume it) register documentationOnly via useShortcutDocs, so the list is complete without relocating any behaviour. Guards. Overlay detection was a hardcoded selector list per handler, already stale: '.modal-backdrop' no longer exists outside a test fixture, and no list knew about the tokenised *-modal__overlay dialogs, the blocking approval prompt included. Layers now register themselves — ModalShell covers every tokenised dialog, and the session popover, which is not a ModalShell, opts in explicitly. Discoverability. '?' opens the sheet, and closes it again — the toggle has to be checked before the overlay guard, because the sheet is itself a layer. '?' stays inert in editable fields, and the composer autofocuses on desktop, so a key-only affordance would not be discoverable at all; the sidebar carries a visible entry point that also works on touch. Labels. comboParts is the only place a combo becomes a keycap, so the New chat tooltip and the sheet cannot drift. use-agent-os#131's hardcoded '⌘⇧O' was wrong on Windows and Linux. Layering. The sheet sits at z-index 60 with the other route dialogs, not on --z-critical-approval, which the approval prompt owns alone and must keep. Pinned by a CSS contract test alongside the existing one. Matching. Combos are matched against both e.key and e.code, so '?' works wherever the layout puts it while Cmd+Shift+O keeps the layout independence the previous e.code === 'KeyO' check gave it. Bundle. The sheet loads lazily: the provider mounts at boot, and pulling ModalShell + motion into the eager tree cost +44 KiB gzip of initial JS for a panel most sessions never open. Split out, the change is +2.0 KiB (132.8 -> 134.8 KiB, budget 180). Gate: tsc, eslint, prettier clean; 1758 frontend tests across 84 files; Control UI build within budget.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #137. Supersedes #209 (closed — the registry shape there was right and is carried forward; the discoverability and layering findings are recorded on that PR).
The console already had real key handling — Enter / Shift+Enter in the composer, Escape's abort/recover chain, Cmd/Ctrl+Shift+O for New chat, arrow navigation in the slash menu and the session switcher — spread across five files as ad-hoc
keydownlisteners. Nothing could enumerate them, so nothing documented them, and each new binding re-litigated the same overlay and editable-target guards.What this does
One registry. Components declare a
ShortcutSpecviauseKeyboardShortcut; the provider owns the single document listener, the editable-target guard and the overlay guard. Keys that must stay bound to their own element — the composer, the slash menu and the session switcher need the event before the document and have to consume it — registerdocumentationOnlyviauseShortcutDocs, so the list is complete without relocating any behaviour. All four files the issue named feed it, plusAppShell's drawer Escape.The overlay guard stopped being a selector list. It was hardcoded per handler and already stale:
.modal-backdropno longer exists outside a test fixture, and no list knew about the tokenised*-modal__overlaydialogs — the blocking approval prompt included. Layers register themselves now (components/overlay-layer.ts):ModalShellcovers every tokenised dialog, and the session popover, which is not aModalShell, opts in explicitly.?is actually reachable. It opens the sheet and closes it again — the close direction has to be checked before the overlay guard, because the sheet is itself a layer. It stays inert in editable fields, and the composer autofocuses on every desktop load (Composer.tsx:319→logic.ts:478-490), so a key-only affordance would not be discoverable at all; the sidebar carries a visible entry point that also works on touch, where there is no keyboard.Labels live in one place.
comboPartsis the only thing that turns a combo into a keycap, so the New chat tooltip and the sheet cannot drift. #131's hardcoded⌘⇧Owas wrong on Windows and Linux.Layering. The sheet sits at
z-index: 60with the other route dialogs, not on--z-critical-approval, which the approval prompt owns alone and must keep — it portals into<body>later, so sharing that token would paint it over a pending approval. Pinned bykeyboard-shortcuts-css.test.tsalongside the existing approval contract.Matching. Combos are matched against both
e.keyande.code, so?works wherever the layout puts it while Cmd+Shift+O keeps the layout independence the previouse.code === 'KeyO'check gave it.Bundle
The sheet loads lazily. The provider mounts at boot, and pulling
ModalShell+motioninto the eager tree cost +44 KiB gzip of initial JS for a panel most sessions never open — under the 180 KiB budget, so the check would not have caught it. Split intoshortcut-combo.ts(pure) /KeyboardShortcuts.tsx(registry) /ShortcutOverlay.tsx(lazy UI), the change is +2.0 KiB (132.8 → 134.8 KiB).Notes for review
ChatPage.test.tsx's "does not start new chat when a modal is open" fabricated a.modal-backdropnode. The guard is registration-based now, so it opens the real session actions menu instead — a stronger assertion than the one it replaces.shift+shiftleft, anduseShortcutDocsre-registered forever when handed an inline array literal (registering re-renders the caller). Both fixed, both covered.Verification
tsc --noEmitclean,eslint srcclean,prettier --check srcclean, 1758 frontend tests across 84 files (basemain: 1738 / 82), Control UI build within budget.