feat(ui): shortcut registry foundation#652
Merged
backnotprop merged 6 commits intomainfrom May 4, 2026
Merged
Conversation
Salvages the foundation of #380 by @gjermundgaraba — the original PR sat 6 weeks waiting for review and was closed by the author. This lands the engine, scope data, and auto-generated marketing docs page, deferring per-component handler migration to follow-up PRs. Layout: - packages/ui/shortcuts/{core,runtime,index}.ts — engine + barrel - packages/ui/shortcuts/plan-review/* — scopes for plan editor surfaces - packages/ui/shortcuts/code-review/* — scopes for review editor surfaces - packages/{editor,review-editor}/shortcuts.ts — per-app surface composition - apps/marketing/src/{components,lib} — auto-generated docs page App behavior is unchanged: existing ad-hoc keydown handlers keep running. The registry is plumbing waiting for migration PRs to wire to it. Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
Contributor
|
All good, I realized it came across a bit passive-agressive, which was not actually the intent 😅. Appreciate the callout ❤️. |
Addresses code review on #652. Adds five live shortcuts that were missing from the registry, so the auto-generated /docs/reference/keyboard-shortcuts page is no longer incomplete: - Mod+P (print plan) — plan review + annotate - Mod+B (toggle file tree) — code review (new "Layout" section) - Mod+. (toggle review sidebar) — code review - Mod+Shift+T (toggle demo tour, dev-only) — code review - Mod+Shift+F (focus PR comments search) — new code-review/prComments scope Also whitelists `.` as a valid normalized binding token in the registry validator (keeps catching typos like Cmd/Ctrl while allowing legitimate punctuation keys). Hold-binding gap: the registry's `Alt hold` advertises a shortcut that `useShortcutScope` cannot dispatch. Today this is harmless because the existing `useInputMethodSwitch` hook handles it bespoke. Added warning comments at the binding declaration and next to `useDoubleTapShortcuts` so future migrators don't fall in. A `useHoldShortcuts` hook is the proper fix and will land with the App.tsx migration that needs it.
Two-Sonnet inventory audit found 19 more user-facing shortcuts not in the registry, on top of the 5 added in the previous commit. Adding all of them so the auto-generated /docs/reference/keyboard-shortcuts page ships honest and the in-app help modal (once migrated to read from the registry) is comprehensive. Plan / annotate surface (4 added): - Escape closes image lightbox (Viewer) - Mod+Enter saves annotation edit (AnnotationPanel) - Escape cancels annotation edit (AnnotationPanel) - Enter confirms image name (ImageAnnotator name field) Code review surface (15 added): - v / a in single-file diff mode (toggle viewed / stage) - v, a, x, z, c, [, ] in all-files (new AllFilesDiff scope) - Mod+Enter / Escape / Tab (new SuggestionModal scope) - Mod+Enter / Escape (new AI Assistant scope: AITab + AskAIInput) - Escape closes tour dialog (new TourDialog scope) Engine: extended NAMED_TOKENS with `[` and `]` for the all-files navigation bindings. Tests updated for the new section ordering. App behavior unchanged — this is pure declarative metadata. The existing ad-hoc keydown handlers keep running; the registry is plumbing for the migration follow-up PRs.
Self-review caught: SuggestionModal.tsx only has Escape (close) and Tab (indent) handlers — no Mod+Enter. The Mod+Enter the inventory agent referenced lives in AnnotationToolbar's suggested-code textarea, which is already covered by reviewAnnotationToolbarShortcuts.submitComment. Removed the bogus submit action from the scope. Renamed cancel→close since the modal preserves suggestion state on Escape (it's not a discard).
The handler at App.tsx:695 checked `e.key === 'c'` but Shift forces `e.key` to uppercase `'C'`, so the shortcut never fired. The in-app help modal also advertised it incorrectly as "Toggle comment mode". Removing the broken shortcut entirely rather than fixing the case mismatch — the toolbar's Copy button already does the same thing, and there's no evidence anyone was relying on the keyboard shortcut. Three deletions: - packages/review-editor/App.tsx — handler block - packages/review-editor/shortcuts.ts — copyDiff registry action - packages/ui/components/KeyboardShortcuts.tsx — help modal entry
- F3: refresh AGENTS.md scope inventory (added scopes were missing)
- F5: getShortcutPlatform() now delegates to packages/ui/utils/platform
isMac instead of duplicating the regex+SSR guard
- F7: drop the stand-alone imageAnnotator.confirmName entry; the
behaviour is folded into the existing save action's hint so the
marketing docs page no longer renders two Enter rows with
different descriptions
- F8: remove the unused listenerOptions parameter from useShortcutScope
(no callers; inline objects in the dep array would have caused
re-render loops if anyone tried to use it)
TODOs left for the migration follow-up:
- F2: cross-scope arbitration — dispatchShortcutEvent should bail on
event.defaultPrevented; needs preventDefault to be claimed
consistently across scopes first
- F6: matchesKeyToken doesn't actually match the Space token; needs a
special case before any scope binds Space
App behaviour unchanged.
3 tasks
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
2 participants
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.
Salvages the foundation of #380 by @gjermundgaraba.
The original PR sat 6 weeks waiting for review and was closed by the author. This PR lands the foundation only — engine, scope data, tests, and the auto-generated marketing docs page — and defers per-component handler migration to follow-up PRs.
What's in this PR
packages/ui/shortcuts/{core,runtime,index}.ts— typed scoped registry with declarative bindings (Mod+Enter,Alt Altdouble-tap,Alt hold), platform-aware formatter (mac glyphs vsCtrl), validator, anduseShortcutScope/useDoubleTapShortcutsReact hooks.packages/ui/shortcuts/plan-review/*— scopes for plan-editor surfaces (annotation toolbar, comment popover, image annotator, viewer, input method).packages/ui/shortcuts/code-review/*— scopes for review-editor surfaces (annotation toolbar, file tree).packages/{editor,review-editor}/shortcuts.ts— each app composes its scopes into aShortcutSurface.apps/marketing/src/{components,lib}—/docs/reference/keyboard-shortcutsis now auto-generated from the registry at build time, replacing the stale 3-shortcut hand-maintained markdown table.packages/ui/shortcuts.test.ts— 264 lines of unit tests covering parser, dispatcher, formatter, registry validation.AGENTS.md(CLAUDE.md) — architecture section + structure tree updated.Layout convention
Subfolder by app:
plan-review/andcode-review/tell you which app's UI a scope serves. Engine lives at the top level since it's truly shared.Behavior change in the apps: none
All existing ad-hoc keydown handlers keep running. The registry is plumbing — nothing wires to it yet.
Verification
bun test— 912 pass, 0 fail (10 new shortcut tests pass)bun run typecheck— clean across all 5 tsconfigsbun run --cwd apps/review build→build:hook→build:opencode→build:marketingFollow-up PRs
KeyboardShortcuts.tsxhelp modal to render from the registry (highest UX-visible win).packages/editor/App.tsxtouseShortcutScope.packages/review-editor/App.tsx.Test plan
bun run dev:hook,bun run dev:review— confirm Cmd+Enter, Cmd+S, Escape, J/K navigation still work/docs/reference/keyboard-shortcutson the built marketing site, confirm rendered outputThank you @gjermundgaraba — sorry for the wait. Co-author credit is in the commit.
Closes #380.