feat(shortcuts): add a keyboard shortcuts dialog (#746)

[Asgabani]

Closes #746.

What changed

• src/lib/shortcuts.ts: a single registry (SHORTCUT_GROUPS) listing every app-wide shortcut — the command palette, Monaco's Run/Format bindings, tab navigation (including the new-tab shortcut, imported from StudioTabBar rather than retyped so it can't drift), and the data profiler's Escape-to-close.
• ShortcutsDialog (src/components/ShortcutsDialog.tsx): a self-contained dialog, following CommandPalette's own Cmd/Ctrl+K effect — it owns its open state and its ? listener (guarded against firing while typing in an input, textarea, or contentEditable element). Exposes an imperative open() via ref, the same seam QueryEditorRef already uses.
• Wired into Studio.tsx directly, plus a new "Keyboard Shortcuts" CommandPalette entry that reaches it through the ref.
• Wired into DataProfiler.tsx directly rather than through props: DataProfiler is rendered by both the standalone shell (Studio.tsx) and the embedded workspace (StudioWorkspace.tsx, which has no CommandPalette), so mounting ShortcutsDialog inside DataProfiler once — scoped to while the profiler is open, matching its own Escape effect's scoping — covers both hosts without threading state through either.

Testing

Ran locally (bun run format && bun run lint && bun run typecheck && bun run knip && bun run chart:check && bun run channels:showcase:check && bun run readme:check && bun run security:check && bun run test && bun run build, plus bun run build:lib && bun run attw since DataProfiler/Sidebar are reachable from the embeddable workspace export surface):

• format / lint / typecheck / knip: clean. Lint warning count unchanged from a clean checkout (191, compared via git stash).
• All four drift guards (chart, channels showcase, readme, security): pass.
• bun run test:components: 44/44 groups pass. (Added the new ShortcutsDialog.test.tsx to tests/run-components.sh's group list — tests/unit/component-runner-coverage.test.ts catches a file missing from it.)
• bun run test: same 13 pre-existing failures as a clean checkout (Helm binary not installed, missing built standalone zip). No live Postgres/MySQL in this sandbox .
• bun run test:coverage && bun run coverage:check: 100.00% line coverage on the merged lcov.
• bun run build, bun run build:lib, bun run attw: all succeed.

If CI surfaces something this sandbox couldn't (Helm chart tests, live DB integration tests), happy to fix it up.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[cevheri]

Thanks @Asgabani, and apologies for the first item: the ground moved under you after you opened this.

main now has src/lib/keyboard-shortcuts.ts, a shared registry with SHORTCUTS, matchesShortcut, shortcutLabel and monacoKeybinding, already used by CommandPalette, QueryEditor and StudioTabBar, and the shortcut line in docs/FEATURES.md is generated from it by bun run shortcuts:sync. That is also your StudioTabBar conflict: main dropped NEW_TAB_SHORTCUT_LABEL for shortcutLabel(SHORTCUTS.newTab). Please rebase and render the dialog from that registry rather than a second one. Adding ? there is not purely mechanical, since every entry feeds the Monaco binding, so use your judgement on the display-only rows. You also get "Cmd/Ctrl+Enter", which your hand-written "Ctrl+Enter" gets wrong on macOS.

Two things I measured that survive the rebase.

? cannot be typed in the query editor. I typed SELECT, pressed ?, and the dialog opened while the character never reached the buffer. Monaco 0.56 focuses a div.native-edit-context, so the input/textarea/contentEditable check misses it, and ? is the positional parameter placeholder in SQLite and MySQL. Your guard tests mount a real textarea, which the editor is no longer.

Studio and DataProfiler each mount a dialog with its own listener, so with the profiler open ? gives you two, and one Escape closes one of them plus the profiler underneath.

A question rather than a change: the tab bar's arrows are listed, the object browser's whole tree pattern is not. Worth settling which side widget keys sit on.

Importing NEW_TAB_SHORTCUT_LABEL was the right instinct, by the way: retyping it and changing the binding does fail your test.

[Asgabani]

Rebased onto `src/lib/keyboard-shortcuts.ts` and fixed both bugs:

Rebase. The four mechanical shortcuts (command palette, run query, format query, new tab) now render via `SHORTCUTS`/`shortcutLabel` instead of a second hand-typed list — you get "Cmd/Ctrl+Enter" etc. correctly now, not my old hardcoded "Ctrl+Enter". I kept `?`, the tab-strip arrow navigation, and the profiler's Escape outside that registry deliberately, per your note that this isn't purely mechanical: none of those three is a Monaco command, and `?` specifically has to exclude the editor rather than bind into it, which is the opposite of what the registry's shape is for (monacoKeybinding is built to hand entries INTO Monaco).

`?` inside Monaco. Your read was right — the guard only checked input/textarea/contentEditable, which Monaco 0.56's edit-context element is none of. Changed it to `target.closest(".monaco-editor") !== null`: checking "anywhere inside the editor" rather than chasing the specific element Monaco happens to focus this version, so it isn't one Monaco upgrade away from being wrong again the same way.

Two dialogs. Confirmed and fixed. Open state now lives in a small module-level store instead of each instance's own `useState`, and only the first-mounted instance actually renders the `Dialog` (promoting to whichever instance is still mounted if that one unmounts) — so `Studio.tsx`'s always-mounted instance and `DataProfiler.tsx`'s conditional one share one dialog and one Escape no matter which combination is in the tree.

Your question on tab-strip vs. object-tree keys: I checked — the tree has no roving-tabindex/arrow-key pattern implemented yet (`TreeRow.tsx`'s only `onKeyDown` is a `stopPropagation` guard on an input), so there's nothing there to list yet. Left it out rather than document a pattern that doesn't exist; happy to add it once the tree has one.

Ran the full local suite again: format/lint/typecheck/knip clean (the `react(set-state-in-effect)` your rebase's own conventions would have caught — the singleton fix originally set state from an effect directly; reworked it through `useSyncExternalStore` instead, same shape `useFavoriteConnections`/`useConnectionOrder` already use), all four drift guards pass, `test:components` 46/46 groups, core suite at the same pre-existing 13 environment-gap failures (Helm/standalone-zip) as a clean checkout, 100.00% coverage, build green.

[cevheri]

Thanks @Asgabani. I ran the new head in the app: ? inside the query editor now reaches the buffer, ? outside it opens the dialog with the registry's labels, and with the profiler open there is exactly one dialog. Two things left, and a correction.

1. One Escape still closes the dialog and the profiler under it. Radix handles Escape in the capture phase and only calls preventDefault(), and the profiler's own listener in DataProfiler.tsx does not check event.defaultPrevented, so it closes too. Returning early there when defaultPrevented is set should do it, with a test that has both open.

2. SHORTCUT_GROUPS is still a second list, so a shortcut added to SHORTCUTS later would not show up in the dialog. A test that every SHORTCUTS entry is listed would hold the two together. Keeping ?, the tab arrows and Escape outside the registry is fine.

On the tree: the keyboard pattern does exist, just not in TreeRow.tsx. ObjectTree.tsx handles it on the role="tree" root: ArrowUp/Down, Home/End, ArrowLeft/Right to collapse and expand, Enter/Space, and Shift+F10 for the row menu. So whether it belongs in the dialog is still an open question.

[Asgabani]

Rebased past #837 (only conflict was tests/run-components.sh, deleted upstream for the new auto-discovering runner — my one-line addition there is moot) and past #849.

1. Escape bug, confirmed and fixed. You had it exactly right — Radix's Dialog handles Escape in the capture phase and only calls preventDefault(), never stopPropagation(), so DataProfiler's own bubble-phase listener still fired. Added a defaultPrevented check, plus a test that opens both and asserts only the shortcuts dialog closes.

2. SHORTCUT_GROUPS drift guard, added. A generic test now iterates Object.values(SHORTCUTS) and asserts every label is listed somewhere in SHORTCUT_GROUPS, so a fifth registry entry with no dialog row fails the suite instead of shipping silently.

3. Object tree keys — thanks for the correction. I'd only checked TreeRow.tsx; ObjectTree.tsx's onKeyDown on the role="tree" root does implement the full pattern (arrows, Home/End, Enter/Space, Shift+F10/ContextMenu). Added an "Object tree" group to the dialog, display-only like the tab-strip arrows — none of it is a Monaco command.

One more thing I updated while I was in there: Studio.tsx, StudioWorkspace.tsx, tests/unit/document-keydown-listeners.test.ts and the D82 docblock in tests/components/studio/embedded-source.test.tsx all manually enumerate every document-level keydown listener with an exact count. ShortcutsDialog.tsx's ? listener makes that five now instead of four, so I updated all four to keep them from going stale the same way the two-listener sentence did.

Full local suite green (targeted runs — this machine is under heavy unrelated load right now, so I didn't trust one wall-clock full run over the isolated file-by-file ones): format/lint/typecheck clean, all the touched test files pass individually, coverage should be unaffected (no new uncovered branches — the defaultPrevented check and the new group are both exercised by the new/existing tests).

[cevheri]

Checked the new head in the app, not only in the tests.

Escape: one press closes the shortcuts dialog and leaves the profiler open, a second closes the profiler. The guard is narrow, it stands down only when something else already handled the key.

Both tests you added because I asked are real. Removing the defaultPrevented line fails your new profiler test and nothing else. Adding a fifth entry to SHORTCUTS with no dialog row fails your generic test while all four of the older ones pass, which is exactly the gap I described, now measured rather than argued.

Also live: with the profiler open there is one dialog, and ? in the query editor reaches the buffer, with document.activeElement the native-edit-context div that defeated the first guard.

Thanks for the object tree group, and for updating the four listener-count sites unasked. That is the kind of enumeration that goes stale in silence.

One line I will adjust myself rather than spend a round trip on: the tree's arrows also move focus, Right into an open row's first child and Left to a closed row's parent, so "collapse / expand" is half of it.

Merging this.

[cevheri]

small note for PR body: "bun run test: same 13 pre-existing failures as a clean checkout (Helm binary not installed, missing built standalone zip). No live Postgres/MySQL in this sandbox."

full test suite working now