Skip to content

Commit 5bc57fc

Browse files
feat(ui): shortcut registry foundation (#652)
* feat(ui): shortcut registry foundation 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> * feat(ui): add missing shortcuts; warn on hold-binding gap 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. * feat(ui): comprehensive shortcut registry coverage 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. * fix(ui): suggestion modal scope advertised non-existent submit shortcut 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). * fix(review): remove broken Cmd+Shift+C copy-diff shortcut 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 * chore(ui): address review nits - 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. --------- Co-authored-by: Gjermund Garaba <gjermund@garaba.net>
1 parent 525c8b0 commit 5bc57fc

29 files changed

Lines changed: 1612 additions & 28 deletions

AGENTS.md

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@ plannotator/
5252
│ │ │ ├── icons/ # Shared SVG icon components (themeIcons, etc.)
5353
│ │ │ ├── plan-diff/ # PlanDiffBadge, PlanDiffViewer, clean/raw diff views
5454
│ │ │ └── sidebar/ # SidebarContainer, SidebarTabs, VersionBrowser, ArchiveBrowser
55+
│ │ ├── shortcuts/ # Keyboard shortcut registry (see Keyboard Shortcuts section below)
56+
│ │ │ ├── core.ts # Engine: parser, formatter, dispatcher, validator
57+
│ │ │ ├── runtime.ts # Engine: useShortcutScope, useDoubleTapShortcuts hooks
58+
│ │ │ ├── index.ts # Barrel — re-exports engine + scopes from both subfolders
59+
│ │ │ ├── plan-review/ # Scopes for plan-editor surfaces (annotationToolbar, annotationPanel, commentPopover, imageAnnotator, inputMethod, viewer)
60+
│ │ │ └── code-review/ # Scopes for review-editor surfaces (ai, allFilesDiff, annotationToolbar, fileTree, prComments, suggestionModal, tourDialog)
61+
│ │ ├── shortcuts.test.ts # Registry unit tests (parser, dispatcher, validator)
5562
│ │ ├── utils/ # parser.ts, sharing.ts, storage.ts, planSave.ts, agentSwitch.ts, planDiffEngine.ts, planAgentInstructions.ts
5663
│ │ ├── hooks/ # useAnnotationHighlighter.ts, useSharing.ts, usePlanDiff.ts, useSidebar.ts, useLinkedDoc.ts, useAnnotationDraft.ts, useCodeAnnotationDraft.ts, useArchive.ts
5764
│ │ └── types.ts
@@ -60,9 +67,12 @@ plannotator/
6067
│ │ ├── storage.ts # Plan saving, version history, archive listing (node:fs only)
6168
│ │ ├── draft.ts # Annotation draft persistence (node:fs only)
6269
│ │ └── project.ts # Pure string helpers (sanitizeTag, extractRepoName, extractDirName)
63-
│ ├── editor/ # Plan review App.tsx
70+
│ ├── editor/ # Plan review app
71+
│ │ ├── App.tsx # Main plan review app
72+
│ │ └── shortcuts.ts # planReviewSurface + annotateSurface — composes plan-review scopes into per-surface registries
6473
│ └── review-editor/ # Code review UI
6574
│ ├── App.tsx # Main review app
75+
│ ├── shortcuts.ts # codeReviewSurface — composes code-review scopes into the review registry
6676
│ ├── components/ # DiffViewer, FileTree, ReviewSidebar
6777
│ ├── dock/ # Dockview center panel infrastructure
6878
│ ├── demoData.ts # Demo diff for standalone mode
@@ -380,6 +390,20 @@ interface Block {
380390

381391
Text highlighting uses `web-highlighter` library. Code blocks use manual `<mark>` wrapping (web-highlighter can't select inside `<pre>`).
382392

393+
## Keyboard Shortcuts
394+
395+
**Location:** `packages/ui/shortcuts/` (engine + scope data), `packages/editor/shortcuts.ts` and `packages/review-editor/shortcuts.ts` (per-app surfaces).
396+
397+
The shortcut system has three layers:
398+
399+
1. **Engine** (`packages/ui/shortcuts/{core,runtime}.ts`) — parser for declarative bindings (`Mod+Enter`, `Alt Alt` double-tap, `Alt hold`), dispatcher, platform-aware formatter (mac glyphs vs. `Ctrl`), validator, and the `useShortcutScope` / `useDoubleTapShortcuts` React hooks. Truly shared — both apps use it as-is.
400+
2. **Scopes**`defineShortcutScope({ id, title, shortcuts: { actionId: { bindings, description, section, ... } } })`. One scope per UI surface (annotation toolbar, comment popover, file tree, etc.). Lives in `packages/ui/shortcuts/{plan-review,code-review}/`**the subfolder names which app's UI the scope serves**. Components/Apps wire handlers to a scope via `useShortcutScope({ scope, handlers: { actionId: () => ... } })`.
401+
3. **Surfaces** (`packages/editor/shortcuts.ts`, `packages/review-editor/shortcuts.ts`) — each app composes its scopes into a `ShortcutSurface` (`planReviewSurface`, `annotateSurface`, `codeReviewSurface`). Surfaces feed both the in-app help modal and the marketing site's auto-generated docs page.
402+
403+
**Convention for adding new shortcuts:** define the action in the relevant scope file under the right subfolder (`plan-review/` or `code-review/`), declare the binding(s) and description, then wire a handler at the call site with `useShortcutScope`. The marketing docs page picks it up automatically at next build. Unit tests in `packages/ui/shortcuts.test.ts` enforce normalized binding tokens (`Mod`, `Shift`, `Alt`, `A-Z`, `1-0`, named keys, `F1``F12`) and unique scope ids.
404+
405+
**Marketing docs auto-generation:** `apps/marketing/src/lib/shortcutReference.ts` reads the three surfaces and `apps/marketing/src/components/ShortcutReference.astro` renders them as tables. The `/docs/reference/keyboard-shortcuts` page is special-cased in `apps/marketing/src/pages/docs/[...slug].astro` to render the component instead of the markdown body.
406+
383407
## URL Sharing
384408

385409
**Location:** `packages/ui/utils/sharing.ts`, `packages/ui/hooks/useSharing.ts`
@@ -482,6 +506,8 @@ Running only `build:opencode` will copy stale HTML files.
482506

483507
`apps/marketing/` is the plannotator.ai website — landing page, documentation, and blog. Built with Astro 5 (static output, zero client JS except a theme toggle island). Docs are markdown files in `src/content/docs/`, blog posts in `src/content/blog/`, both using Astro content collections. Tailwind CSS v4 via `@tailwindcss/vite`. Deploys to S3/CloudFront via GitHub Actions on push to main.
484508

509+
The `/docs/reference/keyboard-shortcuts` page is auto-generated from the shortcut registry at build time — see the Keyboard Shortcuts section above. Editing the markdown body has no effect; update the scope files instead.
510+
485511
## Test plugin locally
486512

487513
```
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
import { shortcutReferenceSurfaces } from '../lib/shortcutReference';
3+
import { formatShortcutBindingsText } from '../../../../packages/ui/shortcuts';
4+
---
5+
6+
<p>Keyboard shortcuts available across the Plannotator review and annotation UIs.</p>
7+
8+
{shortcutReferenceSurfaces.map((surface) => (
9+
<Fragment>
10+
<h2 id={surface.slug}>{surface.title}</h2>
11+
<p>{surface.description}</p>
12+
13+
{surface.sections.map((section) => (
14+
<Fragment>
15+
<h3 id={section.slug}>{section.title}</h3>
16+
<table>
17+
<thead>
18+
<tr>
19+
<th>Shortcut</th>
20+
<th>Action</th>
21+
<th>Notes</th>
22+
</tr>
23+
</thead>
24+
<tbody>
25+
{section.shortcuts.map((shortcut) => (
26+
<tr>
27+
<td><code>{formatShortcutBindingsText(shortcut.bindings)}</code></td>
28+
<td>{shortcut.description}</td>
29+
<td>{shortcut.hint ?? ''}</td>
30+
</tr>
31+
))}
32+
</tbody>
33+
</table>
34+
</Fragment>
35+
))}
36+
</Fragment>
37+
))}

apps/marketing/src/content/docs/reference/keyboard-shortcuts.md

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,14 @@ sidebar:
66
section: "Reference"
77
---
88

9-
Keyboard shortcuts available in the Plannotator plan review, code review, and annotation UIs.
9+
<!--
10+
This page is auto-generated from the shortcut registry at build time.
11+
Editing the body below has no effect — the slug page renders
12+
`apps/marketing/src/components/ShortcutReference.astro` instead.
1013
11-
## Global shortcuts
14+
To change a shortcut, edit the relevant scope file under
15+
`packages/ui/shortcuts/plan-review/` or `packages/ui/shortcuts/code-review/`,
16+
or a per-app surface in `packages/{editor,review-editor}/shortcuts.ts`.
17+
-->
1218

13-
| Shortcut | Context | Action |
14-
|----------|---------|--------|
15-
| `Cmd/Ctrl+Enter` | Plan review (no annotations) | Approve plan |
16-
| `Cmd/Ctrl+Enter` | Plan review (with annotations) | Send feedback |
17-
| `Cmd/Ctrl+Enter` | Code review | Send feedback / Approve |
18-
| `Cmd/Ctrl+Enter` | Annotate mode | Send annotations |
19-
| `Cmd/Ctrl+S` | Any mode (with API) | Quick save to default notes app |
20-
| `Escape` | Annotation toolbar | Close toolbar |
21-
22-
## Notes
23-
24-
- `Cmd/Ctrl+Enter` is blocked when a modal or dialog is open (export, import, confirm dialogs, image annotator)
25-
- `Cmd/Ctrl+Enter` is blocked when typing in an input or textarea
26-
- `Cmd/Ctrl+S` opens the Export modal if no default notes app is configured
27-
- `Escape` in the annotation toolbar closes it without creating an annotation
19+
This page is generated from the shared shortcut registry at build time.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { planReviewSurface, annotateSurface } from '../../../../packages/editor/shortcuts';
2+
import { codeReviewSurface } from '../../../../packages/review-editor/shortcuts';
3+
import { listRegistryShortcutSections } from '../../../../packages/ui/shortcuts';
4+
import type { ShortcutSurface } from '../../../../packages/ui/shortcuts';
5+
6+
const slugify = (value: string) => value.toLowerCase().replace(/\s+/g, '-');
7+
8+
const allSurfaces: ShortcutSurface[] = [planReviewSurface, annotateSurface, codeReviewSurface];
9+
10+
export const shortcutReferenceSurfaces = allSurfaces.map((surface) => ({
11+
...surface,
12+
sections: listRegistryShortcutSections(surface.registry).map((section) => ({
13+
...section,
14+
slug: `${surface.slug}-${slugify(section.title)}`,
15+
})),
16+
}));
17+
18+
export const shortcutReferenceHeadings = shortcutReferenceSurfaces.flatMap((surface) => [
19+
{ depth: 2 as const, slug: surface.slug, text: surface.title },
20+
...surface.sections.map((section) => ({ depth: 3 as const, slug: section.slug, text: section.title })),
21+
]);

apps/marketing/src/pages/docs/[...slug].astro

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
---
22
import { getCollection, render } from 'astro:content';
33
import Docs from '../../layouts/Docs.astro';
4+
import ShortcutReference from '../../components/ShortcutReference.astro';
5+
import { shortcutReferenceHeadings } from '../../lib/shortcutReference';
46
57
export async function getStaticPaths() {
68
const docs = await getCollection('docs');
@@ -11,13 +13,18 @@ export async function getStaticPaths() {
1113
}
1214
1315
const { doc } = Astro.props;
14-
const { Content, headings } = await render(doc);
16+
const isShortcutReference = doc.id === 'reference/keyboard-shortcuts';
17+
const rendered = isShortcutReference ? null : await render(doc);
18+
const Content = rendered?.Content;
19+
const headings = isShortcutReference
20+
? shortcutReferenceHeadings
21+
: rendered?.headings ?? [];
1522
---
1623
<Docs
1724
title={doc.data.title}
1825
description={doc.data.description}
1926
headings={headings}
2027
currentId={doc.id}
2128
>
22-
<Content />
29+
{isShortcutReference ? <ShortcutReference /> : Content && <Content />}
2330
</Docs>

packages/editor/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"type": "module",
55
"exports": {
66
".": "./App.tsx",
7-
"./styles": "./index.css"
7+
"./styles": "./index.css",
8+
"./shortcuts": "./shortcuts.ts"
89
},
910
"dependencies": {
1011
"@plannotator/shared": "workspace:*",

packages/editor/shortcuts.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
import {
2+
annotationPanelShortcuts,
3+
annotationToolbarShortcuts,
4+
commentPopoverShortcuts,
5+
createShortcutRegistry,
6+
createShortcutScopeHook,
7+
defineShortcutScope,
8+
imageAnnotatorShortcuts,
9+
inputMethodShortcuts,
10+
viewerShortcuts,
11+
type ShortcutSurface,
12+
} from '@plannotator/ui/shortcuts';
13+
14+
export const planEditorShortcuts = defineShortcutScope({
15+
id: 'plan-editor',
16+
title: 'Plan Editor',
17+
shortcuts: {
18+
submitPlan: {
19+
description: 'Approve / Send feedback',
20+
bindings: ['Mod+Enter'],
21+
section: 'Actions',
22+
hint: 'Approves when there are no annotations and sends feedback when there are.',
23+
displayOrder: 10,
24+
},
25+
submitAnnotations: {
26+
description: 'Send annotations',
27+
bindings: ['Mod+Enter'],
28+
section: 'Actions',
29+
displayOrder: 10,
30+
},
31+
quickSave: {
32+
description: 'Save to notes app',
33+
bindings: ['Mod+S'],
34+
section: 'Actions',
35+
hint: 'Opens Export if no default notes app is configured.',
36+
displayOrder: 20,
37+
},
38+
exitPlanDiff: {
39+
description: 'Close diff view',
40+
bindings: ['Escape'],
41+
section: 'Actions',
42+
hint: 'Available while plan diff is open.',
43+
displayOrder: 30,
44+
},
45+
printPlan: {
46+
description: 'Print',
47+
bindings: ['Mod+P'],
48+
section: 'Actions',
49+
hint: 'Opens the browser print dialog for the current document.',
50+
displayOrder: 40,
51+
},
52+
},
53+
});
54+
55+
export const usePlanEditorShortcuts = createShortcutScopeHook(planEditorShortcuts);
56+
57+
const planReviewEditorSettingsShortcuts = defineShortcutScope({
58+
id: 'plan-review-editor-settings',
59+
title: 'Plan Editor',
60+
shortcuts: {
61+
submitPlan: planEditorShortcuts.shortcuts.submitPlan,
62+
quickSave: planEditorShortcuts.shortcuts.quickSave,
63+
exitPlanDiff: planEditorShortcuts.shortcuts.exitPlanDiff,
64+
printPlan: planEditorShortcuts.shortcuts.printPlan,
65+
},
66+
});
67+
68+
const annotateEditorSettingsShortcuts = defineShortcutScope({
69+
id: 'annotate-editor-settings',
70+
title: 'Annotate Editor',
71+
shortcuts: {
72+
submitAnnotations: planEditorShortcuts.shortcuts.submitAnnotations,
73+
quickSave: planEditorShortcuts.shortcuts.quickSave,
74+
printPlan: planEditorShortcuts.shortcuts.printPlan,
75+
},
76+
});
77+
78+
const sharedPlanSurfaceShortcuts = [
79+
inputMethodShortcuts,
80+
annotationToolbarShortcuts,
81+
viewerShortcuts,
82+
commentPopoverShortcuts,
83+
annotationPanelShortcuts,
84+
imageAnnotatorShortcuts,
85+
] as const;
86+
87+
export const planReviewSettingsShortcutRegistry = createShortcutRegistry([
88+
planReviewEditorSettingsShortcuts,
89+
...sharedPlanSurfaceShortcuts,
90+
] as const);
91+
92+
export const annotateSettingsShortcutRegistry = createShortcutRegistry([
93+
annotateEditorSettingsShortcuts,
94+
...sharedPlanSurfaceShortcuts,
95+
] as const);
96+
97+
export const planReviewSurface: ShortcutSurface = {
98+
slug: 'plan-review',
99+
title: 'Plan review',
100+
description: 'Shortcuts surfaced by the plan review UI.',
101+
registry: planReviewSettingsShortcutRegistry,
102+
};
103+
104+
export const annotateSurface: ShortcutSurface = {
105+
slug: 'annotate-mode',
106+
title: 'Annotate mode',
107+
description: 'Shortcuts surfaced by the standalone annotation UI.',
108+
registry: annotateSettingsShortcutRegistry,
109+
};

packages/review-editor/App.tsx

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -691,11 +691,6 @@ const ReviewApp: React.FC = () => {
691691
clearSearch();
692692
}
693693
}
694-
// Cmd/Ctrl+Shift+C to copy diff
695-
if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key === 'c') {
696-
e.preventDefault();
697-
handleCopyDiff();
698-
}
699694
// Cmd/Ctrl+B to toggle file tree
700695
if ((e.metaKey || e.ctrlKey) && !e.shiftKey && !e.altKey && e.key.toLowerCase() === 'b' && !isTypingTarget(e.target)) {
701696
e.preventDefault();

packages/review-editor/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
"type": "module",
55
"exports": {
66
".": "./App.tsx",
7-
"./styles": "./index.css"
7+
"./styles": "./index.css",
8+
"./shortcuts": "./shortcuts.ts"
89
},
910
"dependencies": {
1011
"@pierre/diffs": "^1.1.12",

0 commit comments

Comments
 (0)