feat(web): keyboard shortcuts settings page & migrate hotkeys#654
Conversation
Pure functions that lift the existing { key, mod, alt, shift } binding shape
into a canonical 'Mod+Shift+K' string for storage, parse user input from a
capture dialog, and render display tokens for the Kbd component. Mac glyphs
(⌘ ⌥ ⇧ ⎋ ⏎ ⌫ ←→↑↓) vs platform text labels are handled here so callers stay
platform-agnostic.
…rated rows
Adds four new commands (toggleSidebar, closeMediaLightbox, mediaLightboxPrev,
mediaLightboxNext) so the previously hardcoded Sidebar Cmd/Ctrl+B and the
media-lightbox Escape/Arrow keys can flow through the same single-source
binding table the settings page renders. Each row now carries:
- scope 'global' (always live) vs 'mediaLightbox' (component-scoped via
useKeyboardCommand) — drives section grouping on the settings
page and conflict-detection rules.
- i18nKey camelCase id forming 'settings.keyboard.commands.<i18nKey>.label'
and '.description' so translators have a stable surface.
…ct detection The store holds a per-command override map in localStorage and exposes a reactive 'effectiveBindings' that merges those overrides onto the default table. setBinding() rejects invalid combos, OS-reserved combos (Mod+W/Q/T/N), and same-scope collisions; cross-scope collisions are a soft warning since the mediaLightbox scope is only live while the component is mounted. Overrides force browser: 'intercept' on the merged binding so a rebound commands like closeCurrentWorkspaceTab — which is browser: 'passthrough' on its reserved default Mod+W — actually gets dispatched once moved off the reserved combo.
The matcher now continues past bindings whose commands have no live handler so a scoped binding (e.g. mediaLightbox Escape) can hand the key back to a global while its owning component is unmounted, instead of silently swallowing it. connectBrowserKeyboardShortcutsLive takes a bindings getter the listener re-reads on each keydown, so the shortcuts store's effectiveBindings (defaults merged with user overrides) take effect the instant the user rebinds — without churn or re-subscription.
SidebarProvider grows a disableDefaultShortcut prop so consumers that already own a keyboard dispatcher can opt out of the built-in window listener (the default stays false so other consumers — including the dev component wall — keep their behavior unchanged). apps/web's main layout opts in: it disables the provider's listener and binds toggleSidebar through useKeyboardCommand, so the combo is now configurable from the upcoming Keyboard Shortcuts settings page. The master-detail nested layout disables the listener without re-binding — its inner sidebar is layout chrome, not a user-toggleable surface, and shouldn't double-fire the global.
The lightbox's hardcoded window.keydown listener is replaced with three scoped useKeyboardCommand registrations whose handlers return false while the lightbox is closed — the dispatcher then keeps iterating, so a future global binding on the same key isn't shadowed by a deactivated lightbox handler. Focus management (saving the previously focused element on open, restoring on close) stays where it was; only the keyboard wiring moves.
The page lists bindings grouped by scope (Global / Media Lightbox) using the existing SettingsSection layout. Each row shows the i18n label + description and renders the current combo via the design system's Kbd/KbdGroup chips, with a reset icon shown only when the user has overridden the default. KeyCaptureDialog runs a window-level capture-phase keydown listener while open, so any combination — including Escape — is captureable; the listener stopImmediatePropagation()'s past the global dispatcher and reka-ui's escape-to-close. Conflict feedback is reactive: reserved/same-scope blocks Save, cross-scope is allowed with an amber warning. Reset-to-default is exposed inline so users can recover a single binding without nuking the rest.
Adds the /settings/keyboard route (Account group, after Appearance) with the Keyboard lucide icon, plus full en/zh/ja entries for the page chrome, scope labels, conflict-dialog copy, and per-command label/description pairs that the row + dialog read by binding.i18nKey.
…or IPC The desktop renderer now drives its keyboard dispatcher from the shared Pinia store (same as web), so a user override applied in the settings window takes effect the instant the chat window's next keydown fires. A watchEffect on effectiveBindings pushes the menu-delivered commands' Electron accelerators over a new 'desktop:set-menu-accelerators' IPC to the main process; main keeps an overrides Map that takes precedence over the static table when building the native menu, and rebuilds the menu only when the incoming set actually differs. SETTINGS_ROUTE_SPECS gets the /settings/keyboard entry next to Appearance so the desktop settings window can mount the page. The renderer type stubs are extended with the new commands, the KeyboardScope/i18nKey fields, the connectBrowserKeyboardShortcutsLive export, and the @memohai/web/store/keyboard-shortcuts subpath.
Routes the press to the same /settings entry both web and desktop already redirect to bots, so it works as a single-step jump from anywhere — chat, workspace, modal, lightbox — without needing a contextual handler. The combo is non-reserved, so the Keyboard Shortcuts page lets users rebind it like any other row; the existing dispatcher + store wiring carry the rest. The handler is registered directly on the keyboard registry (not via a Vue composable) so it survives every route transition and works in both @memohai/web and the desktop renderer entry without duplicating mount-time plumbing.
- (bug) Sort effectiveBindings so scoped rows precede globals. The first-handled-wins dispatcher previously gave a global the first crack at any combo it shared with a scoped binding; a stable sort fixes the shadowing risk before it bites a future cross-scope collision. - (risk) KeyCaptureDialog now lets Enter/Space pass through when the focused target is a button, so Save/Cancel/Reset stay usable via keyboard. Binding Enter/Space themselves still works as long as focus is off those buttons. - (risk) The desktop renderer's setMenuAccelerators IPC was fire-and-forget; add a .catch that warns so a silent reject doesn't leave the native menu frozen at table defaults with no diagnostic. - (risk) Mod+K open-settings no-ops when already inside /settings, so pressing it from /settings/keyboard doesn't yank the user back to /settings/bots via the default redirect. - (risk) zh.json and ja.json keyboard.intro had a hardcoded English 'Edit'; swap in 「编辑」/「編集」 so the prose matches the localized button label.
…ator The earlier menu-accelerator override commit added a helper typed `(command: AppKeyboardCommand)` but only imported the runtime values `acceleratorForCommand` and `appKeyboardCommands`. tsc --noEmit (Electron CI's typecheck:node step) errors with TS2304 'Cannot find name AppKeyboardCommand'. Add the type import so the desktop main process typecheck is clean again.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Memoh/apps/web/src/lib/keyboard-bindings.ts
Line 163 in 699c707
Normalize DOM key names before Electron accelerators
When a desktop user rebinds the menu-delivered close-tab command to keys captured from KeyboardEvent.key, values like ArrowLeft or the space character are sent directly to Electron. Electron accelerator key codes use names such as Left/Right and Space, not DOM names like ArrowLeft or raw ' ', so these saved menu shortcuts silently lose their native accelerator even though the settings UI accepts them. Please translate captured DOM keys to Electron accelerator names before rebuilding the menu.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mod: isMac ? event.metaKey : event.ctrlKey, | ||
| alt: event.altKey, | ||
| shift: event.shiftKey, | ||
| key: canonicalKey(event.key), |
There was a problem hiding this comment.
On macOS, capturing a shortcut such as Control+S reaches this path with isMac === true, so ctrlKey is discarded and the stored combo becomes plain s. The global dispatcher then matches that plain letter anywhere, including while typing in normal inputs, so a user trying to bind a Ctrl-based shortcut can accidentally make ordinary text entry trigger the command. Please either model Ctrl separately or reject/ignore captures that contain unsupported modifier keys instead of dropping them.
Useful? React with 👍 / 👎.
| useKeyboardCommand(appKeyboardCommands.toggleSidebar, () => { | ||
| isOpen.value = !isOpen.value | ||
| return true |
There was a problem hiding this comment.
In the desktop shell this layout explicitly skips auto-collapse because the settings sidebar is meant to stay pinned open, but the new global toggle still flips isOpen. Pressing the default Mod+B while a desktop settings page is focused now collapses that sidebar despite the desktop layout having no collapse affordance and assuming it remains open; guard this handler with desktopShell or avoid registering it for the desktop settings layout.
Useful? React with 👍 / 👎.
| </div> | ||
| <div | ||
| v-else-if="conflict.kind === 'cross-scope'" | ||
| class="text-amber-600 dark:text-amber-400" |
There was a problem hiding this comment.
apps/web/AGENTS.md requires semantic color tokens and explicitly forbids hardcoded Tailwind colors for themed elements. This warning state hardcodes amber shades with a dark: override, so it can drift from Memoh color schemes and violates the documented UI contract; use an existing semantic token or add a token for this warning color.
Useful? React with 👍 / 👎.
…arning token - keyComboFromEvent rejects any macOS capture with ctrlKey set. Previously ctrl was silently dropped to mod=false, so binding Ctrl+S on macOS would store as plain 's' and the dispatcher would fire on every literal 's' keypress — including normal typing. - KeyCaptureDialog's cross-scope warning swaps the hardcoded amber Tailwind classes for the existing semantic --warning token so it tracks Memoh color schemes and stops violating apps/web/AGENTS.md's UI contract. The third review note (Mod+B toggling the desktop chat sidebar) was not addressed: that behavior was already present in the pre-migration SidebarProvider's unconditional keydown listener, so the centralization preserves rather than introduces it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c36ae5854
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (combo.mod) parts.push('Mod') | ||
| if (combo.alt) parts.push('Alt') | ||
| if (combo.shift) parts.push('Shift') | ||
| parts.push(combo.key === ' ' ? 'Space' : combo.key) |
There was a problem hiding this comment.
When the user captures the literal + key, formatKeyCombo persists strings like + or Mod+Shift++, but parseKeyCombo later splits on + and filters empty tokens, so applyOverride treats the saved shortcut as invalid. The dialog appears to save the binding, yet the effective binding immediately falls back to the default and remains broken after reload; serialize plus as a distinct token such as Plus, or make the parser round-trip it.
Useful? React with 👍 / 👎.
| const overrides: Record<string, string> = {} | ||
| for (const binding of shortcutsStore.effectiveBindings) { | ||
| if (binding.desktop !== 'menu') continue | ||
| overrides[binding.command] = toElectronAccelerator(binding) |
There was a problem hiding this comment.
When a desktop user rebinds the menu-delivered close-tab command to an arrow key, this sends DOM key names such as ArrowLeft through toElectronAccelerator. The Electron accelerator docs list arrow key codes as Left/Right/Up/Down, so the native menu accelerator for that override is invalid and the shortcut will not work in desktop. Map captured KeyboardEvent.key values to Electron accelerator key names before pushing them to the main process.
Useful? React with 👍 / 👎.
| if (conflict.kind === 'same-scope' || conflict.kind === 'reserved') return conflict | ||
| overrides.value = { ...overrides.value, [command]: formatKeyCombo(parsed) } |
There was a problem hiding this comment.
For global commands this accepts any non-reserved combo, including a bare printable key such as b or an unmodified arrow key. The window-level dispatcher does not skip focused inputs or contenteditable areas, and handlers like toggleSidebar/openSettings return true, so after such a binding, typing that key in a form or editor prevents text entry and runs the command. Require a modifier for global shortcuts, or ignore unmodified global matches from editable targets.
Useful? React with 👍 / 👎.
| useKeyboardCommand(appKeyboardCommands.toggleSidebar, () => { | ||
| isOpen.value = !isOpen.value | ||
| return true | ||
| }) |
There was a problem hiding this comment.
This is the only toggleSidebar handler registered in the app, but MainLayout is mounted only for the settings overlay; the normal / and /bot chat shell uses components/sidebar/index.vue and useWorkspaceTabsStore().toggleWorkbench() instead. On chat routes the advertised global Mod+B/custom binding dispatches with no live handler and does nothing, so register the command while MainSection is active as well.
Useful? React with 👍 / 👎.
| if (binding.command === command) continue | ||
| if (!keyCombosEqual(comboFromBinding(binding), combo)) continue | ||
| if (binding.scope === ownBinding.scope) return { kind: 'same-scope', collidesWith: binding.command } | ||
| return { kind: 'cross-scope', collidesWith: binding.command } |
There was a problem hiding this comment.
When a combo collides with both a scoped binding and an existing global override, this returns cross-scope on the first scoped match and never checks for the later same-scope collision. For example, after Save active file is rebound to ArrowLeft (allowed because it first collides with media previous), rebinding Toggle sidebar to ArrowLeft is also allowed, leaving two global commands on the same key and making the dispatcher claim whichever one happens to handle first. Keep scanning and let any same-scope collision block the save.
Useful? React with 👍 / 👎.
…+ global rules Five issues from the second review batch, all accepted: - (memohai#4) keyboard-combo encodes literal '+' as the 'Plus' token both ways. 'Shift++' split on '+' to ['Shift','',''] and filtered to ['Shift'], silently corrupting the saved binding into an unparseable string; the dialog appeared to save but the override fell back to the default. - (memohai#5) toElectronAccelerator now maps DOM key names (ArrowLeft/Escape/Space) to Electron's accelerator names (Left/Esc/Space). Pushing the raw DOM name produced an invalid accelerator string that the native menu silently rejected, so a rebound close-tab to Mod+ArrowLeft would never fire. - (memohai#6) Global bindings now require Cmd/Ctrl or Alt. The window-level dispatcher does not skip focused inputs, so a bare-key global like 'b' would fire every literal keystroke and break text entry. Scoped bindings keep accepting bare keys because their handlers only register while the owning component is mounted. A new 'no-modifier' ConflictKind and three-language error string surface the rule in the capture dialog. - (memohai#7 + the earlier memohai#2) toggleSidebar moves from main-layout (settings overlay) to main-section (chat shell). MainLayout was only mounted on /settings, so chat routes had no live handler and Mod+B silently fell through to the browser; MainSection is persistent, so the handler runs on both chat and settings. We no-op on settings routes to preserve the desktop sidebar's pinned-open intent AND claim the press so the browser doesn't see Mod+B. - (memohai#8) detectConflict now scans all colliding bindings before deciding, so a same-scope match found later still blocks the save even when a cross-scope match was iterated first.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8b0185c8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // user picks a Cmd/Alt/Shift-based combo instead. | ||
| if (isMac && event.ctrlKey) return null | ||
| return { | ||
| mod: isMac ? event.metaKey : event.ctrlKey, |
There was a problem hiding this comment.
The macOS Ctrl guard fixes one unsupported modifier path, but on Windows/Linux metaKey is still ignored here. If a user captures a Super/Windows-key combo that also has Alt (for example Super+Alt+K) or a scoped key like Super+ArrowLeft, the stored binding drops Meta and becomes Alt+K/bare ArrowLeft; the dispatcher then triggers on that easier combo and will not match the original Meta combo. Reject non-macOS metaKey captures or model Super separately instead of silently downgrading them.
Useful? React with 👍 / 👎.
| if (binding.scope === ownBinding.scope) return { kind: 'same-scope', collidesWith: binding.command } | ||
| crossScopeMatch = crossScopeMatch ?? binding.command | ||
| } | ||
| if (crossScopeMatch) return { kind: 'cross-scope', collidesWith: crossScopeMatch } |
There was a problem hiding this comment.
When desktop users assign the menu-delivered Close current workspace tab command to a combo that also belongs to a media-lightbox command (for example after rebinding both to Mod+Shift+P), this cross-scope result is allowed. The scoped-first dispatch ordering only applies to renderer keydown bindings, but close tab is excluded from that listener and is fired by Electron's native menu accelerator, so pressing the combo with a lightbox open can close the workspace tab instead of letting the lightbox claim it. Treat cross-scope collisions involving desktop: 'menu' as blocking or route them through the same dispatcher.
Useful? React with 👍 / 👎.
| ArrowUp: 'Up', | ||
| ArrowDown: 'Down', | ||
| Escape: 'Esc', | ||
| ' ': 'Space', |
There was a problem hiding this comment.
Fresh evidence beyond the earlier plus persistence issue is that the new Electron key-name map still omits the literal + key. After a desktop user rebinds the menu-delivered close-tab command to + (or Shift++), toElectronAccelerator emits strings such as CmdOrCtrl++ / CmdOrCtrl+Shift++; Electron accelerator strings use + as the separator and list the key as Plus, so the native menu shortcut can be rejected or fail to match even though the setting was saved. Map '+' to Plus before rebuilding the menu.
Useful? React with 👍 / 👎.
2 participants
Summary