Add global keyboard shortcuts with hints and a shortcuts sheet #351
- MA
## Why Keyboard users had no shell shortcuts or place to discover them.
## What changed - Add one tested shortcut registry for Search, New document, Projects sidebar, Chat, and the keyboard shortcuts sheet. - Show platform-aware hints in tooltips, sidebar actions, and the sheet. Load shortcut handling after first paint and the sheet when opened. - Suppress plain-key shortcuts while typing or using a menu; leave editor-owned chords and open modals alone. - Match the character a layout types, including shifted punctuation. Fall back to physical keys only when the layout does not report an ASCII character. Review follow-ups cover Dvorak, AZERTY, non-Latin layouts, and touch hints.
## Verification The 14 focused shortcut tests, `bun run types`, and `bun run ci` passed locally on the rebased commits. Required GitHub checks will run on the final published head.
## Visuals 


MaggieAppletonhead-ref-force-pushed bd074dc4de9elast month on GitHub
- MA
**Review:** Changes needed
Visual check done (headless Chromium, 1440x900, phone, Mac and Linux UA). The sheet, hint chips, tooltip suffix and account item look right. One real bug and a few smaller points.
**Blocking** 1. `apps/web/src/shortcuts.ts` `matchShortcut`: the `code` fallback hijacks real shortcuts on remapped layouts. `Cmd+.` also matches on `event.code === "Period"`, but on Dvorak the physical Period key types `v`, so `Cmd+V` (paste) toggles Chat and is `preventDefault`ed. Reproduced: a keydown `{key:"v", code:"Period", metaKey:true}` dispatched from a focused editor returns `defaultPrevented === true`. Fix: trust `event.key` first; use `code` only when `key` is not an ASCII letter or digit (dead key, `Unidentified`, non-Latin layout). Add a unit test for Dvorak `v` on Period. 2. Same function: the shift check runs before both matches, so on layouts where `.` or `\` needs Shift (AZERTY), `Cmd+Shift+;` (key `.`) never matches, while the unshifted physical key (printed `:`) does by `code`. The label says `⌘.` but the user must press `:`. Fixing (1) mostly resolves this; also stop requiring `!shiftKey` for `key`-matched chords.
**Non-blocking** 3. Only "Hide sidebar" carries a shortcut tooltip. The collapsed-state "Show sidebar" button has no `data-tooltip-shortcut`, so the chord is taught half the time. 4. Touch: the account menu always shows "Keyboard shortcuts" with a `?` kbd on phones and tablets without a keyboard. Hide the `kbd` (and consider the item) under `(hover: none)` like `.project-sidebar-hint`. 5. `workspace.tsx` effect deps `[!chat, paperObscured]` are cryptic; name `let enabled = !!chat && !paperObscured` and depend on that.
**Checked and fine** - No hijacking while typing: `c` and `?` typed in the chat composer insert text and fire nothing. Chords (`Cmd+K`, `Cmd+\`, `Cmd+.`) work from the composer; closing Chat from the composer keeps focus on the toggle. An open dialog suppresses all shortcuts (`Cmd+K` inside the sheet is ignored). Composition (`isComposing`) and Alt/AltGr are ignored; `Cmd+K` with a selection still goes to the link editor. - Mac matches Meta only and others Ctrl only; labels read `⌘K` / `Ctrl+K`, `Ctrl Shift Z`. Only the standard browser `Ctrl+K` override. - The sheet reuses the palette shell and reads as its sibling (same width, search field, hairline rows, keycaps). Hint chips are invisible at rest, appear on hover/focus in quiet ink, and are hidden under `hover: none`. Tooltip `Hide sidebar ⌘\` is subdued and consistent. - Unit tests (11) pass locally. I built with the raw bundle budget raised in my own worktree only (nothing pushed). I did not run `e2e/keyboard-shortcuts.e2e.ts`: ports 8797 and 5434 are held by other sessions.
MaggieAppletonhead-ref-force-pushed 5ffc21e70163last month on GitHub
MaggieAppletonbase-ref-changedlast month on GitHub
- MA
Thanks. I've fixed all five points in 5ffc21e7:
1. `matchShortcut` now matches `event.key` first. The physical `code` is only a fallback when the key is not an ASCII letter or digit. Search also has a `KeyK` fallback for non-Latin layouts. New unit tests cover Dvorak: `{key:"v", code:"Period", meta}` no longer matches, and Dvorak `⌘.` (physical E) does. In the browser, the synthetic Dvorak ⌘V keydown from the editor now returns `defaultPrevented === false`, and Chat stays open. 2. Shift now distinguishes only letter chords. `.`, `\` and `?` match on `event.key` whether or not Shift is held. AZERTY tests cover `⌘⇧;` → `.` → Chat and `?`. 3. The collapsed "Show sidebar" button now has the chord. The label comes from the lazy shortcut module, so the registry stays out of the initial chunk. Checked in the browser ("Show sidebar ⌘\") and added to the e2e. 4. On touch devices (`(hover: none)`), the account menu hides the `?` kbd. 5. The Chat effect now depends on a named `chatShortcutEnabled = !!chat && !paperObscured`.
The PR is now stacked on #354. With that headroom, the initial JS is 247893 B raw, within budget, so CI can run the browser suite.
- MA
**Review:** Changes needed
Visual check done (headless Chromium, 1440x900 plus phone and Linux UA, main vs PR). Sheet, hint chips, tooltip and account item look right. One real bug and a few smaller points.
**Blocking** 1. `apps/web/src/shortcuts.ts` matchShortcut: the `code` fallback hijacks real shortcuts on remapped layouts. `Cmd+.` also matches on `event.code === "Period"`, but on Dvorak the physical Period key types `v`, so `Cmd+V` (paste) toggles Chat and is `preventDefault`ed. I reproduced this: a keydown `{key:"v", code:"Period", metaKey:true}` from a focused editor returns `defaultPrevented === true`. Fix: trust `event.key` first; only use `code` when `key` is not a letter or digit (for example a dead key, `Unidentified`, or a non-Latin character from a layout with no ASCII). Add a unit test for Dvorak `v`/Period. 2. Same area: the shift check runs before both matches, so on layouts where `.` or `\` needs Shift (AZERTY), `Cmd+Shift+;` (key `.`) never matches, while the unshifted physical key (`:`) does. The label says `⌘.` but the user must press a key printed `:`. Ignore `shiftKey` for `key`-matched chords (only require it for the `code` path to stay unshifted), or drop the `code` path once (1) is fixed.
**Non-blocking** 3. Sidebar hint is only on "Hide sidebar". The collapsed-state "Show sidebar" button has no `data-tooltip-shortcut`, so the chord is taught only half the time. Add it there. 4. Touch: the account menu always shows "Keyboard shortcuts ?" and the `?` kbd on phones/tablets without a keyboard. Hide the `kbd` under `(hover: none)` like `.project-sidebar-hint` (and consider hiding the item). 5. `workspace.tsx` effect deps `[!chat, paperObscured]` are cryptic; name a `let enabled = !!chat && !paperObscured` and depend on that.
**Checked and fine** - No hijacking while typing: `c`/`?` typed in the chat composer and editor insert text and fire nothing; chords (`Cmd+K`, `Cmd+\`, `Cmd+.`) work from the composer, with focus kept (chat close restores focus to the toggle). Open dialogs suppress all shortcuts (`Cmd+K` inside the sheet is ignored). `isComposing` and Alt/AltGr are ignored; a synthetic composing `c` did nothing. `Cmd+K` with a selection still goes to the link editor. - Mac uses Meta only and others Ctrl only; labels are `⌘K` / `Ctrl+K`, and `Ctrl+Shift+Z` form on Linux. No clash with browser or OS chords beyond the usual `Ctrl+K` (browser search) override, which is standard. - Sheet reuses the palette shell and reads as a sibling: same width, search field, rows with hairlines and keycaps. Hint chips are invisible at rest, show on hover/focus, quiet ink, and are hidden under `hover: none`. Tooltip suffix `Hide sidebar ⌘\` is subdued and consistent. - Unit tests (11) pass locally. e2e file reads well but I did not run it: ports 8797 and 5434 are held by other sessions, so I skipped it per the brief. I built with the raw bundle budget raised locally in my worktree only (nothing pushed).
CI at review time: `format, lint, types, tests` fail (not yet inspected, logs unavailable while the run is in progress), `e2e` pending, `container` pending.
MaggieAppletonbase-ref-force-pushedlast month on GitHub
MaggieAppletonautomatic-base-change-succeededlast month on GitHub
MaggieAppletonreferenced this in Prefetch the sign-in page and keep stale routes from canonicalizing #362last month on GitHub
MaggieAppletonreferenced this in Move signed-out and lazy-only code out of the initial bundle #354last month on GitHub
MaggieAppletonhead-ref-force-pushed 03fdb8a363c3last month on GitHub
MaggieAppletonhead-ref-force-pushed aadfbf70a2aelast month on GitHub
MaggieAppletonhead-ref-force-pushed 23c7cd9e0520last month on GitHub
MaggieAppletonauto-rebase-enabledlast month on GitHub
MaggieAppletonmerged 838a26c77faalast month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub