refactor(desktop): rewrite hotkey system with react-hotkeys-hook#3178
Merged
saddlepaddle merged 17 commits intomainfrom Apr 5, 2026
Merged
refactor(desktop): rewrite hotkey system with react-hotkeys-hook#3178saddlepaddle merged 17 commits intomainfrom
saddlepaddle merged 17 commits intomainfrom
Conversation
New hotkey module at renderer/hotkeys/ with:
- Registry: explicit per-platform key definitions (mac/windows/linux)
- Hooks: useHotkey (listener), useHotkeyDisplay (display), useRecordHotkeys (settings recording)
- Store: Zustand + localStorage persist for user overrides
- Display: formatHotkeyDisplay returning { keys, text } for Kbd components and tooltip text
- Components: HotkeyLabel for rendering label + shortcut badge
- Utils: isTerminalReservedEvent for xterm handler
Old system in shared/hotkeys.ts + stores/hotkeys/ not yet removed — migration pending.
16 files updated: - useAppHotkey → useHotkey from renderer/hotkeys - HotkeyTooltipContent → HotkeyLabel - useHotkeyText → useHotkeyDisplay - Removed deps arrays and undefined option placeholders
…mplify menu/xterm - Migrate all display-only files to new hotkey system (useHotkeyDisplay, HotkeyLabel) - Delete old stores/hotkeys/ and components/HotkeyTooltipContent/ - Remove useHotkeysSync from authenticated layout - Hardcode Electron menu accelerators (no longer customizable) - Simplify xterm handler to use getBinding + isTerminalReservedEvent - Remove trpcHotkeysStorage adapter - Add migration script for old hotkey overrides
- Replace manual recording logic with useRecordHotkeys hook - Derive hotkeys-by-category from HOTKEYS registry (no store hook) - Remove import/export functionality - All renderer code now uses renderer/hotkeys exclusively
- Add OPEN_SETTINGS and SHOW_HOTKEYS back to registry (used by renderer) - Remove stores/hotkeys re-export from stores/index.ts - Migrate 3 files using useHotkeysStore for platform to PLATFORM const - Remove leftover deps arrays from 2 chat search hooks - Make HotkeyLabel id prop optional for conditional rendering
… routers) - Delete shared/hotkeys.ts (1042 lines) and its test file - Delete routers/hotkeys/ (import/export router) - Delete hotkeys-events.ts (emitter for menu sync) - Strip hotkeys set/subscribe from ui-state router (keep read-only get for migration) - Inline legacy hotkeys type in app-state schemas
…hotkeys-hook xterm uses a textarea for keyboard input, which causes react-hotkeys-hook to ignore events (it skips form tags by default). Instead of enabling form tags globally, dispatch a clean synthetic KeyboardEvent on document when the xterm handler detects a ctrl/meta combo that should be an app hotkey. The synthetic event's target is document (not textarea), so react-hotkeys-hook processes it normally.
…bility react-hotkeys-hook matches against event.code (not event.key), so special characters need to use their code names: - "/" → "slash" - "," → "comma" - "[" → "bracketleft" - "]" → "bracketright" Added corresponding display mappings so they render correctly in the UI.
…patch - Add enableOnFormTags: true as default in useHotkey so hotkeys work when focused in textareas (chat input, etc.) - Remove synthetic event dispatch from xterm handler — no longer needed since react-hotkeys-hook now processes form tag events directly - Fix key format: use code-based names (slash, comma, bracketleft/right) for react-hotkeys-hook compatibility
Replace react-hotkeys-hook's useRecordHotkeys (caused infinite update loop from reactive Set state) with a simple window keydown listener in useEffect. Same approach as the old settings page, but extracted as a reusable hook.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDeleted legacy shared/main hotkeys and TRPC sync; added a renderer-only hotkeys subsystem (registry, types, display, hooks, override store, migration, UI components), replaced renderer usages to the new APIs, and added dependency Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant useHotkey as useHotkey Hook
participant useBinding as useBinding
participant Store as Overrides Store
participant Registry as HOTKEYS
participant ReactHotkeys as react-hotkeys-hook
participant Keyboard as KeyboardEvent
Component->>useHotkey: call useHotkey("ID", callback, options)
useHotkey->>useBinding: resolve binding for "ID"
useBinding->>Store: read overrides
alt override exists
Store-->>useBinding: return override
else
Registry-->>useBinding: return default key
end
useBinding-->>useHotkey: resolved key string
useHotkey->>ReactHotkeys: register handler for resolved key
Keyboard-->>ReactHotkeys: key pressed
ReactHotkeys-->>Component: invoke callback
useHotkey-->>Component: returns HotkeyDisplay {keys, text}
sequenceDiagram
participant User
participant RecorderUI as Recording UI
participant useRecord as useRecordHotkeys
participant Store as Overrides Store
participant Registry as HOTKEYS
User->>RecorderUI: start recording for ID
RecorderUI->>useRecord: attach keydown listener
User->>useRecord: press combination
useRecord->>useRecord: normalize + validate
useRecord->>Registry: check conflicts
alt conflict
useRecord-->>RecorderUI: onConflict(conflictId)
else reserved-warning
useRecord-->>RecorderUI: onReserved(warning)
else valid
useRecord->>Store: setOverride(ID, keys)
Store-->>localStorage: persist
useRecord-->>RecorderUI: onSave()
end
Estimated Code Review Effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
8 issues found across 83 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts">
<violation number="1" location="apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts:13">
P1: Normalize `event.key` before composing the hotkey string; raw browser key values can mismatch registry token names and break conflict/default/reserved comparisons.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/layout.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/layout.tsx:71">
P2: Handle the async migration promise explicitly in the effect to avoid unhandled promise rejections.
(Based on your team's feedback about handling async calls/errors explicitly.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/hotkeys/components/HotkeyLabel/HotkeyLabel.tsx">
<violation number="1" location="apps/desktop/src/renderer/hotkeys/components/HotkeyLabel/HotkeyLabel.tsx:6">
P1: Passing `""` as a fallback hotkey id can crash at runtime (`HOTKEYS[id]` is undefined). Use a valid fallback id instead of an invalid cast.</violation>
</file>
<file name="apps/desktop/src/renderer/hotkeys/utils/utils.ts">
<violation number="1" location="apps/desktop/src/renderer/hotkeys/utils/utils.ts:15">
P2: Backslash matching is over-escaped, so `Ctrl+\\` never matches the terminal-reserved set.</violation>
</file>
<file name="apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts">
<violation number="1" location="apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts:14">
P1: The hotkey callback is memoized with incomplete dependencies, which can invoke stale closures when component state changes.</violation>
</file>
<file name="apps/desktop/src/renderer/hotkeys/registry.ts">
<violation number="1" location="apps/desktop/src/renderer/hotkeys/registry.ts:38">
P2: Navigation hotkeys are assigned to a category that the keyboard settings UI does not render, so they won’t appear for users.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx:131">
P2: Backspace/Delete unassign now resets to default instead of disabling the shortcut.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx:168">
P2: Conflict reassignment no longer warns when the chosen key is OS-reserved.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const MODIFIER_ORDER = ["meta", "ctrl", "alt", "shift"] as const; | ||
|
|
||
| function captureHotkeyFromEvent(event: KeyboardEvent): string | null { | ||
| const key = event.key.toLowerCase(); |
Contributor
There was a problem hiding this comment.
P1: Normalize event.key before composing the hotkey string; raw browser key values can mismatch registry token names and break conflict/default/reserved comparisons.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts, line 13:
<comment>Normalize `event.key` before composing the hotkey string; raw browser key values can mismatch registry token names and break conflict/default/reserved comparisons.</comment>
<file context>
@@ -0,0 +1,146 @@
+const MODIFIER_ORDER = ["meta", "ctrl", "alt", "shift"] as const;
+
+function captureHotkeyFromEvent(event: KeyboardEvent): string | null {
+ const key = event.key.toLowerCase();
+ if (["shift", "ctrl", "alt", "meta", "dead", "unidentified"].includes(key))
+ return null;
</file context>
| import type { HotkeyId } from "../../registry"; | ||
|
|
||
| export function HotkeyLabel({ label, id }: { label: string; id?: HotkeyId }) { | ||
| const { keys } = useHotkeyDisplay(id ?? ("" as HotkeyId)); |
Contributor
There was a problem hiding this comment.
P1: Passing "" as a fallback hotkey id can crash at runtime (HOTKEYS[id] is undefined). Use a valid fallback id instead of an invalid cast.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/hotkeys/components/HotkeyLabel/HotkeyLabel.tsx, line 6:
<comment>Passing `""` as a fallback hotkey id can crash at runtime (`HOTKEYS[id]` is undefined). Use a valid fallback id instead of an invalid cast.</comment>
<file context>
@@ -0,0 +1,18 @@
+import type { HotkeyId } from "../../registry";
+
+export function HotkeyLabel({ label, id }: { label: string; id?: HotkeyId }) {
+ const { keys } = useHotkeyDisplay(id ?? ("" as HotkeyId));
+ if (!id || keys[0] === "Unassigned") return <span>{label}</span>;
+ return (
</file context>
| linux: "ctrl+shift+bracketleft", | ||
| }, | ||
| label: "Navigate Back", | ||
| category: "Navigation", |
Contributor
There was a problem hiding this comment.
P2: Navigation hotkeys are assigned to a category that the keyboard settings UI does not render, so they won’t appear for users.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/hotkeys/registry.ts, line 38:
<comment>Navigation hotkeys are assigned to a category that the keyboard settings UI does not render, so they won’t appear for users.</comment>
<file context>
@@ -0,0 +1,576 @@
+ linux: "ctrl+shift+bracketleft",
+ },
+ label: "Navigate Back",
+ category: "Navigation",
+ description: "Go back to the previous page in history",
+ },
</file context>
| toast.warning("This shortcut may be reserved by your OS."); | ||
| } | ||
| setOverride(pendingConflict.conflictId, null); | ||
| setOverride(pendingConflict.targetId, pendingConflict.keys); |
Contributor
There was a problem hiding this comment.
P2: Conflict reassignment no longer warns when the chosen key is OS-reserved.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx, line 168:
<comment>Conflict reassignment no longer warns when the chosen key is OS-reserved.</comment>
<file context>
@@ -132,118 +156,16 @@ function KeyboardShortcutsPage() {
- toast.warning("This shortcut may be reserved by your OS.");
- }
+ setOverride(pendingConflict.conflictId, null);
+ setOverride(pendingConflict.targetId, pendingConflict.keys);
setPendingConflict(null);
};
</file context>
| useRecordHotkeys(recordingId, { | ||
| onSave: () => setRecordingId(null), | ||
| onCancel: () => setRecordingId(null), | ||
| onUnassign: () => setRecordingId(null), |
Contributor
There was a problem hiding this comment.
P2: Backspace/Delete unassign now resets to default instead of disabling the shortcut.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx, line 131:
<comment>Backspace/Delete unassign now resets to default instead of disabling the shortcut.</comment>
<file context>
@@ -92,34 +85,65 @@ export const Route = createFileRoute("/_authenticated/settings/keyboard/")({
+ useRecordHotkeys(recordingId, {
+ onSave: () => setRecordingId(null),
+ onCancel: () => setRecordingId(null),
+ onUnassign: () => setRecordingId(null),
+ onConflict: (targetId, keys, conflictId) => {
+ setPendingConflict({ targetId, keys, conflictId });
</file context>
Suggested change
| onUnassign: () => setRecordingId(null), | |
| onUnassign: (id) => { | |
| setOverride(id, null); | |
| setRecordingId(null); | |
| }, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx (1)
79-91:⚠️ Potential issue | 🟠 MajorSame stale closure concern for
CLOSE_WORKSPACEcallback.The callback references
currentWorkspaceIdandcurrentWorkspace, which are captured at registration time. When the hotkey fires later, these values may be stale.Since
enabledalready guards activation via!!currentWorkspaceId, you could read the current workspace inside the callback or use a ref.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx` around lines 79 - 91, The CLOSE_WORKSPACE hotkey callback closes over currentWorkspaceId and currentWorkspace at registration time causing stale values; change the callback to read the latest workspace state when invoked (or use a ref) instead of relying on the closed-over variables. Specifically, update the useHotkey registration for "CLOSE_WORKSPACE" so the handler queries the current workspace (e.g., from the same state selector or a currentWorkspaceRef.current) and then calls setDeleteTarget({ workspaceId: latestId, workspaceName: latest.name, workspaceType: latest.type }) only when that fresh value exists; keep the enabled: !!currentWorkspaceId guard but do not rely on those closed-over variables inside the callback.
🧹 Nitpick comments (7)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/NavigationControls/NavigationControls.tsx (1)
15-16: Optional: guard hotkey actions with navigation availability.To mirror button disabled behavior, consider no-op’ing when back/forward is unavailable.
♻️ Suggested refinement
- useHotkey("NAVIGATE_BACK", () => router.history.back()); - useHotkey("NAVIGATE_FORWARD", () => router.history.forward()); + useHotkey("NAVIGATE_BACK", () => { + if (!canGoBack) return; + router.history.back(); + }); + useHotkey("NAVIGATE_FORWARD", () => { + if (!canGoForward) return; + router.history.forward(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/NavigationControls/NavigationControls.tsx` around lines 15 - 16, The hotkey handlers call router.history.back() and router.history.forward() unconditionally; update the useHotkey callbacks for "NAVIGATE_BACK" and "NAVIGATE_FORWARD" to first check that navigation is available and no-op otherwise (e.g., test a capability on router.history such as a canGoBack/canGoForward method if present, or check history length/index before calling router.history.back() / router.history.forward()). Modify the useHotkey callbacks in NavigationControls.tsx to guard the calls to router.history.back and router.history.forward so they mirror the buttons' disabled behavior.apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.ts (1)
7-9: Consider usingHotkeyIdtype instead ofstringfor better type safety.The function accepts
stringbut immediately casts it toParameters<typeof useBinding>[0](which is likelyHotkeyId). AcceptingHotkeyIddirectly would provide compile-time validation at call sites and eliminate the need for the cast.♻️ Proposed fix
+import type { HotkeyId } from "../../types"; + -export function useHotkeyDisplay(id: string): HotkeyDisplay { - const binding = useBinding(id as Parameters<typeof useBinding>[0]); +export function useHotkeyDisplay(id: HotkeyId): HotkeyDisplay { + const binding = useBinding(id); return useMemo(() => formatHotkeyDisplay(binding, PLATFORM), [binding]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.ts` around lines 7 - 9, Change the useHotkeyDisplay parameter from a plain string to the specific HotkeyId type so callers get compile-time validation and you can remove the cast to Parameters<typeof useBinding>[0]; update the function signature for useHotkeyDisplay(id: HotkeyId): HotkeyDisplay, adjust any necessary imports (e.g., HotkeyId) and then call useBinding(id) directly (no cast) and keep the useMemo(formatHotkeyDisplay(binding, PLATFORM), [binding]) logic as-is.apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.ts (1)
21-28: Hardcoded⌘symbol may be inconsistent with user overrides and platform.The
workspaceShortcutLabelsmap uses a hardcoded⌘${index + 1}format, but the new hotkey system supports user overrides and platform-specific display formatting viauseHotkeyDisplay. If a user customizes these workspace shortcuts, this label won't reflect their changes.Consider using
useHotkeyDisplayfor each workspace shortcut to maintain consistency with the rest of the hotkey system, or verify this is intentional given the new architecture.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.ts` around lines 21 - 28, The workspaceShortcutLabels map currently hardcodes labels as `⌘${index + 1}` which ignores user overrides and platform formatting; update the logic in useDashboardSidebarShortcuts to compute each label using the hotkey formatting utility instead of a literal string: for each workspace in flattenedWorkspaces.slice(0, MAX_SHORTCUT_COUNT) call useHotkeyDisplay (or the project's equivalent) with the same accelerator/keybinding used when registering the workspace shortcut and use its returned string as the map value (keep keys as workspace.id); ensure this replaces the hardcoded template so labels reflect user and platform-specific formatting.apps/desktop/src/renderer/components/HotkeyMenuShortcut/HotkeyMenuShortcut.tsx (1)
10-14: Consider using a dedicated flag instead of string comparison for "Unassigned" check.Comparing against the magic string
"Unassigned"is fragile—if the display text changes, this check would silently break. Consider havinguseHotkeyDisplayreturn anisAssignedboolean alongsidetext, or exporting theUNASSIGNED_TEXTconstant from the hotkeys module for consistent comparison.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/HotkeyMenuShortcut/HotkeyMenuShortcut.tsx` around lines 10 - 14, The component HotkeyMenuShortcut currently checks for the magic string "Unassigned" from useHotkeyDisplay; change the implementation so the hook returns a stable indicator (e.g., { text, isAssigned }) or export a shared UNASSIGNED_TEXT constant and use that instead of a string literal; update HotkeyMenuShortcut to read isAssigned (or compare against UNASSIGNED_TEXT) and return null when not assigned, ensuring you modify useHotkeyDisplay (and the hotkeys module if adding a constant) and update all callers to use the new boolean/constant.apps/desktop/src/renderer/hotkeys/components/HotkeyLabel/HotkeyLabel.tsx (1)
12-14: Potential duplicate React keys ifkeysarray contains repeated values.Using
k(the key name string) as the Reactkeycould cause warnings if the same modifier appears twice in the display. Consider using the index or a composite key:♻️ Suggested fix
<KbdGroup> - {keys.map((k) => ( - <Kbd key={k}>{k}</Kbd> + {keys.map((k, i) => ( + <Kbd key={`${k}-${i}`}>{k}</Kbd> ))} </KbdGroup>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/components/HotkeyLabel/HotkeyLabel.tsx` around lines 12 - 14, The HotkeyLabel component uses keys.map((k) => <Kbd key={k}>{k}</Kbd>), which can produce duplicate React keys when the same key string repeats; change the map to include the index or a composite key (e.g., use keys.map((k, i) => <Kbd key={`${k}-${i}`}>{k}</Kbd>)) so each Kbd has a unique key and React warnings are avoided.apps/desktop/src/renderer/hotkeys/display.ts (1)
1-5: Minor documentation inconsistency: "mod" alias not handled.The file docstring mentions
"mod+ctrl+enter"as an example input, but"mod"isn't included inMODIFIER_ORDERorMODIFIER_DISPLAY. Ifreact-hotkeys-hookuses"mod"as a platform-agnostic alias formeta(mac) orctrl(windows/linux), the formatter should handle it; otherwise, the docstring example should be updated.💡 Option A: Update docstring to reflect actual supported format
/** * Display formatting for hotkey bindings. - * Converts key strings like "mod+ctrl+enter" into platform-specific symbols. + * Converts key strings like "meta+shift+n" into platform-specific symbols. */💡 Option B: Add "mod" alias support if needed
-const MODIFIER_ORDER = ["meta", "ctrl", "alt", "shift"] as const; +const MODIFIER_ORDER = ["meta", "ctrl", "alt", "shift", "mod"] as const; const MODIFIER_DISPLAY: Record<Platform, Record<string, string>> = { - mac: { meta: "⌘", ctrl: "⌃", alt: "⌥", shift: "⇧" }, - windows: { meta: "Win", ctrl: "Ctrl", alt: "Alt", shift: "Shift" }, - linux: { meta: "Super", ctrl: "Ctrl", alt: "Alt", shift: "Shift" }, + mac: { meta: "⌘", ctrl: "⌃", alt: "⌥", shift: "⇧", mod: "⌘" }, + windows: { meta: "Win", ctrl: "Ctrl", alt: "Alt", shift: "Shift", mod: "Ctrl" }, + linux: { meta: "Super", ctrl: "Ctrl", alt: "Alt", shift: "Shift", mod: "Ctrl" }, };Also applies to: 31-31
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/display.ts` around lines 1 - 5, Docstring mentions "mod" but the code doesn't handle that alias; add support for "mod" by treating it as a platform-agnostic alias that resolves to "meta" on macOS and "ctrl" on Windows/Linux: update MODIFIER_DISPLAY to include a "mod" entry (or add a small normalization step) and ensure MODIFIER_ORDER includes "mod" where appropriate, and update the hotkey parsing/formatting path (the function that builds display strings using MODIFIER_ORDER/MODIFIER_DISPLAY) to normalize input keys by expanding "mod" to the correct platform-specific modifier before rendering; also adjust the file docstring example if you instead prefer to document the actual supported tokens.apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx (1)
27-33: Keep the category list exhaustive.
getHotkeysByCategory()builds aNavigationbucket, but both filtering and rendering only iterateCATEGORY_ORDER, which omits that category. AnyHOTKEYSentry assigned toNavigationwill silently disappear from this page. Either include the missing category here or derive the rendered order from the grouped data so these lists cannot drift.Also applies to: 88-113, 148-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx` around lines 27 - 33, CATEGORY_ORDER currently omits the "Navigation" category so any HOTKEYS assigned to that category are dropped during rendering; update the code to either add "Navigation" to CATEGORY_ORDER (so CATEGORY_ORDER: HotkeyCategory[] includes "Navigation") or change the rendering/filtering to derive the display order from the keys of getHotkeysByCategory(HOTKEYS) (i.e., iterate the grouped object’s keys instead of the static CATEGORY_ORDER) and ensure getHotkeysByCategory, CATEGORY_ORDER, and the render loop remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts`:
- Around line 14-16: The hotkey registration currently only depends on keys
which causes stale closures; update the useHotkeys call in useHotkey (the line
invoking useHotkeys(keys ?? "", callback, { enableOnFormTags: true, ...options
}, [keys])) to include callback in the dependency array (e.g., [keys, callback])
so the binding is re-registered when the handler changes, and note that callers
should memoize callback (useCallback) to avoid excessive re-registrations.
In `@apps/desktop/src/renderer/hotkeys/utils/utils.ts`:
- Line 8: The TERMINAL_RESERVED set entry "ctrl+\\" doesn't match the value
produced when handling event.key because the ternary produces "ctrl+\\\\" for a
single backslash key; update the logic so the produced key string and the set
entries use the same representation: either change the set entry to match the
doubled-backslash form or (preferred) remove the special-case ternary and
normalize backslash handling so the set contains "ctrl+\\" (single escaped
backslash literal) and the code that builds the key string uses that same form;
ensure you update the entries referenced (e.g., "ctrl+\\" in TERMINAL_RESERVED)
and the event.key branch that currently emits "\\\\" to stay consistent.
In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx`:
- Around line 62-68: The hotkey callback currently closes over a stale
isWorkspaceSidebarOpen value because useHotkey only depends on keys; update the
callback to read the latest state at invocation (instead of closing over the
variable) by either calling the store getter/read function or using a ref that
you update on state changes. Concretely: in the hotkey handler used with
useHotkey, replace direct use of isWorkspaceSidebarOpen with a live read (e.g.,
call the workspace store getter or selector to get current
isWorkspaceSidebarOpen) or maintain a latestIsWorkspaceSidebarOpen ref updated
in an effect and read that ref inside the callback; continue to call
setWorkspaceSidebarOpen(true) or toggleWorkspaceSidebarCollapsed() as before.
In `@apps/desktop/src/renderer/routes/_authenticated/layout.tsx`:
- Around line 69-72: The effect calls migrateHotkeyOverrides() without handling
rejections, so failures are silent; update the useEffect in layout.tsx to call
migrateHotkeyOverrides() inside an async wrapper (or .then/.catch) and
explicitly handle errors: await the promise returned by
migrateHotkeyOverrides(), catch any thrown error, and log or surface it (e.g.,
processLogger.error / console.error and optionally set a UI-visible flag) so
migration failures are not silent. Ensure you reference the existing useEffect
and migrateHotkeyOverrides function names and do not change their signatures,
just add proper async handling and error handling inside the effect.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx`:
- Around line 189-195: The per-row "Reset" handler does not clear the global
recording state, so if a row is reset while showing "Recording…" the key
listener remains armed; update the per-row reset handlers (the Button onClick
that calls resetAll in the bulk path and the single-row reset handler used
around lines 236-243) to also call setRecordingId(null) when performing a reset
(i.e., ensure setRecordingId(null) is invoked alongside the reset logic that
clears the binding), so the recordingId is cleared for both the global "Reset
all" path and each per-row reset.
---
Outside diff comments:
In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx`:
- Around line 79-91: The CLOSE_WORKSPACE hotkey callback closes over
currentWorkspaceId and currentWorkspace at registration time causing stale
values; change the callback to read the latest workspace state when invoked (or
use a ref) instead of relying on the closed-over variables. Specifically, update
the useHotkey registration for "CLOSE_WORKSPACE" so the handler queries the
current workspace (e.g., from the same state selector or a
currentWorkspaceRef.current) and then calls setDeleteTarget({ workspaceId:
latestId, workspaceName: latest.name, workspaceType: latest.type }) only when
that fresh value exists; keep the enabled: !!currentWorkspaceId guard but do not
rely on those closed-over variables inside the callback.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/HotkeyMenuShortcut/HotkeyMenuShortcut.tsx`:
- Around line 10-14: The component HotkeyMenuShortcut currently checks for the
magic string "Unassigned" from useHotkeyDisplay; change the implementation so
the hook returns a stable indicator (e.g., { text, isAssigned }) or export a
shared UNASSIGNED_TEXT constant and use that instead of a string literal; update
HotkeyMenuShortcut to read isAssigned (or compare against UNASSIGNED_TEXT) and
return null when not assigned, ensuring you modify useHotkeyDisplay (and the
hotkeys module if adding a constant) and update all callers to use the new
boolean/constant.
In `@apps/desktop/src/renderer/hotkeys/components/HotkeyLabel/HotkeyLabel.tsx`:
- Around line 12-14: The HotkeyLabel component uses keys.map((k) => <Kbd
key={k}>{k}</Kbd>), which can produce duplicate React keys when the same key
string repeats; change the map to include the index or a composite key (e.g.,
use keys.map((k, i) => <Kbd key={`${k}-${i}`}>{k}</Kbd>)) so each Kbd has a
unique key and React warnings are avoided.
In `@apps/desktop/src/renderer/hotkeys/display.ts`:
- Around line 1-5: Docstring mentions "mod" but the code doesn't handle that
alias; add support for "mod" by treating it as a platform-agnostic alias that
resolves to "meta" on macOS and "ctrl" on Windows/Linux: update MODIFIER_DISPLAY
to include a "mod" entry (or add a small normalization step) and ensure
MODIFIER_ORDER includes "mod" where appropriate, and update the hotkey
parsing/formatting path (the function that builds display strings using
MODIFIER_ORDER/MODIFIER_DISPLAY) to normalize input keys by expanding "mod" to
the correct platform-specific modifier before rendering; also adjust the file
docstring example if you instead prefer to document the actual supported tokens.
In
`@apps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.ts`:
- Around line 7-9: Change the useHotkeyDisplay parameter from a plain string to
the specific HotkeyId type so callers get compile-time validation and you can
remove the cast to Parameters<typeof useBinding>[0]; update the function
signature for useHotkeyDisplay(id: HotkeyId): HotkeyDisplay, adjust any
necessary imports (e.g., HotkeyId) and then call useBinding(id) directly (no
cast) and keep the useMemo(formatHotkeyDisplay(binding, PLATFORM), [binding])
logic as-is.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.ts`:
- Around line 21-28: The workspaceShortcutLabels map currently hardcodes labels
as `⌘${index + 1}` which ignores user overrides and platform formatting; update
the logic in useDashboardSidebarShortcuts to compute each label using the hotkey
formatting utility instead of a literal string: for each workspace in
flattenedWorkspaces.slice(0, MAX_SHORTCUT_COUNT) call useHotkeyDisplay (or the
project's equivalent) with the same accelerator/keybinding used when registering
the workspace shortcut and use its returned string as the map value (keep keys
as workspace.id); ensure this replaces the hardcoded template so labels reflect
user and platform-specific formatting.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/NavigationControls/NavigationControls.tsx`:
- Around line 15-16: The hotkey handlers call router.history.back() and
router.history.forward() unconditionally; update the useHotkey callbacks for
"NAVIGATE_BACK" and "NAVIGATE_FORWARD" to first check that navigation is
available and no-op otherwise (e.g., test a capability on router.history such as
a canGoBack/canGoForward method if present, or check history length/index before
calling router.history.back() / router.history.forward()). Modify the useHotkey
callbacks in NavigationControls.tsx to guard the calls to router.history.back
and router.history.forward so they mirror the buttons' disabled behavior.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx`:
- Around line 27-33: CATEGORY_ORDER currently omits the "Navigation" category so
any HOTKEYS assigned to that category are dropped during rendering; update the
code to either add "Navigation" to CATEGORY_ORDER (so CATEGORY_ORDER:
HotkeyCategory[] includes "Navigation") or change the rendering/filtering to
derive the display order from the keys of getHotkeysByCategory(HOTKEYS) (i.e.,
iterate the grouped object’s keys instead of the static CATEGORY_ORDER) and
ensure getHotkeysByCategory, CATEGORY_ORDER, and the render loop remain
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dbc50899-b4e3-49c2-9f8b-6c53767d0fc9
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (82)
apps/desktop/package.jsonapps/desktop/src/lib/trpc/routers/hotkeys/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/ui-state/index.tsapps/desktop/src/main/lib/app-state/schemas.tsapps/desktop/src/main/lib/hotkeys-events.tsapps/desktop/src/main/lib/menu.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/components/Chat/ChatInterface/components/ChatInputFooter/components/ChatShortcuts/ChatShortcuts.tsxapps/desktop/src/renderer/components/HotkeyMenuShortcut/HotkeyMenuShortcut.tsxapps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsxapps/desktop/src/renderer/components/HotkeyTooltipContent/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/renderer/hooks/useWorkspaceShortcuts.tsapps/desktop/src/renderer/hotkeys/components/HotkeyLabel/HotkeyLabel.tsxapps/desktop/src/renderer/hotkeys/components/HotkeyLabel/index.tsapps/desktop/src/renderer/hotkeys/display.tsapps/desktop/src/renderer/hotkeys/hooks/index.tsapps/desktop/src/renderer/hotkeys/hooks/useBinding/index.tsapps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.tsapps/desktop/src/renderer/hotkeys/hooks/useHotkey/index.tsapps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.tsapps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/index.tsapps/desktop/src/renderer/hotkeys/hooks/useHotkeyDisplay/useHotkeyDisplay.tsapps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/index.tsapps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.tsapps/desktop/src/renderer/hotkeys/index.tsapps/desktop/src/renderer/hotkeys/migrate.tsapps/desktop/src/renderer/hotkeys/registry.tsapps/desktop/src/renderer/hotkeys/stores/hotkeyOverridesStore.tsapps/desktop/src/renderer/hotkeys/stores/index.tsapps/desktop/src/renderer/hotkeys/types.tsapps/desktop/src/renderer/hotkeys/utils/index.tsapps/desktop/src/renderer/hotkeys/utils/utils.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarHeader/DashboardSidebarHeader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarExpandedWorkspaceRow/DashboardSidebarExpandedWorkspaceRow.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceHoverCardContent/DashboardSidebarWorkspaceHoverCardContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/NavigationControls/NavigationControls.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OrganizationDropdown/OrganizationDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/SearchBarTrigger/SearchBarTrigger.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/SidebarToggle/SidebarToggle.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/WorkspaceRunButton/WorkspaceRunButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/TasksTopBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/CreateTaskDialog/CreateTaskDialog.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceEmptyState/WorkspaceEmptyState.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatInputFooter/ChatInputFooter.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatInputFooter/components/ChatShortcuts/ChatShortcuts.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/WorkspaceChatInterface/components/ChatMessageList/hooks/useChatMessageSearch/useChatMessageSearch.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/hooks/usePresetHotkeys.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/SidebarControl/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/components/WorkspaceHoverCard/WorkspaceHoverCard.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/NewWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPaneInterface/components/ChatMessageList/hooks/useChatMessageSearch/useChatMessageSearch.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useDiffSearch/useDiffSearch.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/FileViewerPane/hooks/useMarkdownSearch/useMarkdownSearch.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/components/PaneToolbarActions/PaneToolbarActions.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/ScrollToBottomButton/ScrollToBottomButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalHotkeys.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PaneContextMenuItems/PaneContextMenuItems.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/components/PresetsBar/components/PresetBarItem/PresetBarItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/index.tsxapps/desktop/src/renderer/stores/hotkeys/index.tsapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/shared/hotkeys.test.tsapps/desktop/src/shared/hotkeys.ts
💤 Files with no reviewable changes (11)
- apps/desktop/src/lib/trpc/routers/index.ts
- apps/desktop/src/renderer/components/HotkeyTooltipContent/index.ts
- apps/desktop/src/main/lib/hotkeys-events.ts
- apps/desktop/src/renderer/stores/hotkeys/index.ts
- apps/desktop/src/renderer/stores/index.ts
- apps/desktop/src/renderer/lib/trpc-storage.ts
- apps/desktop/src/lib/trpc/routers/hotkeys/index.ts
- apps/desktop/src/shared/hotkeys.test.ts
- apps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsx
- apps/desktop/src/renderer/stores/hotkeys/store.ts
- apps/desktop/src/shared/hotkeys.ts
Comment on lines
+62
to
+68
| useHotkey("TOGGLE_WORKSPACE_SIDEBAR", () => { | ||
| if (!isWorkspaceSidebarOpen) { | ||
| setWorkspaceSidebarOpen(true); | ||
| } else { | ||
| toggleWorkspaceSidebarCollapsed(); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
Callback may capture stale isWorkspaceSidebarOpen state.
The useHotkey hook (per useHotkey.ts lines 14-16) only includes keys in its dependency array, not the callback. This means isWorkspaceSidebarOpen is captured at registration time and won't reflect updates.
Consider using a ref pattern or ensuring the callback reads from the store directly:
🛠️ Suggested fix using store getter
useHotkey("TOGGLE_WORKSPACE_SIDEBAR", () => {
- if (!isWorkspaceSidebarOpen) {
+ const isOpen = useWorkspaceSidebarStore.getState().isOpen;
+ if (!isOpen) {
setWorkspaceSidebarOpen(true);
} else {
toggleWorkspaceSidebarCollapsed();
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx` around
lines 62 - 68, The hotkey callback currently closes over a stale
isWorkspaceSidebarOpen value because useHotkey only depends on keys; update the
callback to read the latest state at invocation (instead of closing over the
variable) by either calling the store getter/read function or using a ref that
you update on state changes. Concretely: in the hotkey handler used with
useHotkey, replace direct use of isWorkspaceSidebarOpen with a live read (e.g.,
call the workspace store getter or selector to get current
isWorkspaceSidebarOpen) or maintain a latestIsWorkspaceSidebarOpen ref updated
in an effect and read that ref inside the callback; continue to call
setWorkspaceSidebarOpen(true) or toggleWorkspaceSidebarCollapsed() as before.
Greptile SummaryThis PR replaces ~1,400 lines of custom key-parsing, matching, and normalization with a registry-based hotkey system backed by
Confidence Score: 3/5Not safe to merge — two P1 bugs cause immediate user-facing failures in the keyboard shortcuts settings page and wherever HotkeyLabel is rendered without an id. The architecture is solid and code quality is high throughout, but the conflict detector in useRecordHotkeys.ts is context-unaware and will break the shortcuts UI for five groups of shared-binding hotkeys, and HotkeyLabel crashes with a TypeError when rendered without an id prop. Both fixes are small but are required before merging. useRecordHotkeys.ts (conflict detection logic), HotkeyLabel.tsx and useBinding.ts (crash path when id is undefined) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User clicks hotkey row] --> B[setRecordingId]
B --> C[User presses keys]
C --> D[captureHotkeyFromEvent]
D --> E{Bare modifier or\nunrecognized key?}
E -- yes --> C
E -- no --> F[checkReserved]
F --> G{Terminal reserved?}
G -- error --> H[onReserved - abort]
G -- no --> I{OS reserved?}
I -- warning --> J[onReserved toast - continue]
I -- no --> J
J --> K[getHotkeyConflict\nFLAT SCAN over all hotkeys]
K --> L{Conflict found?}
L -- yes --> M[onConflict - show dialog]
M --> N{User choice}
N -- Reassign --> O[setOverride conflictId null\nsetOverride targetId keys]
N -- Cancel --> P[No change]
L -- no --> Q{captured === defaultKey?}
Q -- yes --> R[resetOverride]
Q -- no --> S[setOverride]
R --> T[onSave - setRecordingId null]
S --> T
T --> U[useBinding re-reads store]
U --> V[useHotkeys re-registers]
Reviews (1): Last reviewed commit: "fix: add addEventListener mock to test s..." | Re-trigger Greptile |
… not hotkey system)
- Fix HotkeyLabel crash when id is undefined (add ?. guard in useBinding) - Fix backslash over-escaping in isTerminalReservedEvent - Fix stale closure in useHotkey (add callbackRef pattern) - Fix Backspace/Delete to disable hotkey (set null) instead of resetting to default - Handle migration promise rejection explicitly - Cancel recording when per-row Reset is clicked
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts (2)
1-2: Prefer tsconfig alias imports instead of deep relative paths.These imports should use the configured alias form to match project conventions and reduce path fragility.
As per coding guidelines,
apps/desktop/**/*.{ts,tsx}: Use alias as defined intsconfig.jsonwhen possible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts` around lines 1 - 2, Imports in useBinding.ts use deep relative paths; update the import statements that bring in HOTKEYS, HotkeyId and useHotkeyOverridesStore to use the project's tsconfig path aliases instead of "../../registry" and "../../stores/hotkeyOverridesStore". Locate the import lines that reference HOTKEYS and HotkeyId and the one that references useHotkeyOverridesStore and replace their module specifiers with the corresponding tsconfig alias modules (the alias used for the renderer hotkeys registry and stores) so they conform to the project's alias convention.
5-19: Deduplicate binding resolution between hook and imperative helper.
useBindingandgetBindingcurrently duplicate the same resolution branch logic. Extracting a shared resolver will prevent drift.♻️ Suggested refactor
+function resolveBinding( + id: HotkeyId, + overrides: Record<string, string | null>, +): string | null { + if (!id) return null; + if (id in overrides) return overrides[id] ?? null; + return HOTKEYS[id]?.key ?? null; +} + /** Reactive: get the effective key binding for a hotkey (override ?? default) */ export function useBinding(id: HotkeyId): string | null { - return useHotkeyOverridesStore((state) => { - if (!id) return null; - if (id in state.overrides) return state.overrides[id] ?? null; - return HOTKEYS[id]?.key ?? null; - }); + return useHotkeyOverridesStore((state) => resolveBinding(id, state.overrides)); } /** Imperative: get the effective key binding (for non-React contexts like xterm) */ export function getBinding(id: HotkeyId): string | null { - const state = useHotkeyOverridesStore.getState(); - if (!id) return null; - if (id in state.overrides) return state.overrides[id] ?? null; - return HOTKEYS[id]?.key ?? null; + const { overrides } = useHotkeyOverridesStore.getState(); + return resolveBinding(id, overrides); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts` around lines 5 - 19, Extract the duplicate resolution logic used in useBinding and getBinding into a single shared resolver (e.g., resolveBinding or getEffectiveBinding) that takes a HotkeyId and an overrides map (or the store state) and returns the resolved key or null; update useBinding to call useHotkeyOverridesStore with a selector that delegates to this shared resolver, and update getBinding to read the store via useHotkeyOverridesStore.getState() and delegate to the same resolver so both functions use identical logic and avoid drift.apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx (2)
27-33: Keep category ordering in one source of truth.
getHotkeysByCategory()builds aNavigationbucket, but the page only filters and renders categories fromCATEGORY_ORDER. Anything categorized asNavigationwill silently disappear from this screen.♻️ Simple fix
const CATEGORY_ORDER: HotkeyCategory[] = [ + "Navigation", "Workspace", "Terminal", "Layout", "Window", "Help", ];Also applies to: 88-113, 148-159, 216-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx` around lines 27 - 33, The page filters and renders categories using the hardcoded CATEGORY_ORDER which omits the "Navigation" bucket produced by getHotkeysByCategory(), causing those hotkeys to be hidden; update the rendering to use a single source of truth by either adding "Navigation" to CATEGORY_ORDER or (preferably) deriving the category list from the keys returned by getHotkeysByCategory() and then applying your existing ordering as a stable fallback, ensuring references to CATEGORY_ORDER, getHotkeysByCategory, and the "Navigation" category are synchronized so no category is dropped.
165-169: Reuse the same default-vs-override normalization when resolving conflicts.The direct recording path removes the override when the chosen keys equal
HOTKEYS[id].key, but this branch always persists a string. That leaves a redundant override behind when the user reassigns a shortcut back to its default.♻️ Keep the conflict path consistent with the recorder
const handleConflictReassign = () => { if (!pendingConflict) return; setOverride(pendingConflict.conflictId, null); - setOverride(pendingConflict.targetId, pendingConflict.keys); + if (pendingConflict.keys === HOTKEYS[pendingConflict.targetId].key) { + resetOverride(pendingConflict.targetId); + } else { + setOverride(pendingConflict.targetId, pendingConflict.keys); + } setPendingConflict(null); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx` around lines 165 - 169, The conflict-resolution path in handleConflictReassign currently always writes a string override for pendingConflict.targetId, leaving a redundant override when the chosen keys equal the default; change the logic in handleConflictReassign so it uses the same normalization as the recorder: if pendingConflict.keys equals HOTKEYS[pendingConflict.targetId].key call setOverride(pendingConflict.targetId, null), otherwise call setOverride(pendingConflict.targetId, pendingConflict.keys); keep the existing setOverride(pendingConflict.conflictId, null) and setPendingConflict(null) behavior.apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts (1)
2-4: Use therenderer/*alias for these internal hotkey imports.The new hotkeys module is already using alias-based imports elsewhere in this PR; keeping these as
../../...makes the tree more brittle to future moves.As per coding guidelines, "Use alias as defined in
tsconfig.jsonwhen possible".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts` around lines 2 - 4, Replace the relative imports with the renderer alias imports: import HOTKEYS, HotkeyId, and PLATFORM from the renderer hotkeys registry and import useHotkeyOverridesStore and Platform from their renderer stores/types instead of using "../../registry", "../../stores/hotkeyOverridesStore", and "../../types"; update the import statements that reference HOTKEYS, type HotkeyId, PLATFORM, useHotkeyOverridesStore, and type Platform to use the "renderer/..." path aliases defined in tsconfig so the module uses the alias-based imports consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts`:
- Around line 12-31: In captureHotkeyFromEvent, normalize event.key before
building the shortcut string: after const key = event.key.toLowerCase(); map
known aliases (e.g. map "control" -> "ctrl" and the literal " " -> "space")
using a small keyAliases lookup and replace key with the normalized value; then
proceed with the existing exclusion list, F-key test, modifier checks, and final
join so you avoid producing duplicates like "ctrl+control" and so space-based
shortcuts become "space" (and thus match reserved combos like "meta+space");
reference function captureHotkeyFromEvent and constants MODIFIER_ORDER and
PLATFORM when making the change.
---
Nitpick comments:
In `@apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.ts`:
- Around line 1-2: Imports in useBinding.ts use deep relative paths; update the
import statements that bring in HOTKEYS, HotkeyId and useHotkeyOverridesStore to
use the project's tsconfig path aliases instead of "../../registry" and
"../../stores/hotkeyOverridesStore". Locate the import lines that reference
HOTKEYS and HotkeyId and the one that references useHotkeyOverridesStore and
replace their module specifiers with the corresponding tsconfig alias modules
(the alias used for the renderer hotkeys registry and stores) so they conform to
the project's alias convention.
- Around line 5-19: Extract the duplicate resolution logic used in useBinding
and getBinding into a single shared resolver (e.g., resolveBinding or
getEffectiveBinding) that takes a HotkeyId and an overrides map (or the store
state) and returns the resolved key or null; update useBinding to call
useHotkeyOverridesStore with a selector that delegates to this shared resolver,
and update getBinding to read the store via useHotkeyOverridesStore.getState()
and delegate to the same resolver so both functions use identical logic and
avoid drift.
In
`@apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts`:
- Around line 2-4: Replace the relative imports with the renderer alias imports:
import HOTKEYS, HotkeyId, and PLATFORM from the renderer hotkeys registry and
import useHotkeyOverridesStore and Platform from their renderer stores/types
instead of using "../../registry", "../../stores/hotkeyOverridesStore", and
"../../types"; update the import statements that reference HOTKEYS, type
HotkeyId, PLATFORM, useHotkeyOverridesStore, and type Platform to use the
"renderer/..." path aliases defined in tsconfig so the module uses the
alias-based imports consistently.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx`:
- Around line 27-33: The page filters and renders categories using the hardcoded
CATEGORY_ORDER which omits the "Navigation" bucket produced by
getHotkeysByCategory(), causing those hotkeys to be hidden; update the rendering
to use a single source of truth by either adding "Navigation" to CATEGORY_ORDER
or (preferably) deriving the category list from the keys returned by
getHotkeysByCategory() and then applying your existing ordering as a stable
fallback, ensuring references to CATEGORY_ORDER, getHotkeysByCategory, and the
"Navigation" category are synchronized so no category is dropped.
- Around line 165-169: The conflict-resolution path in handleConflictReassign
currently always writes a string override for pendingConflict.targetId, leaving
a redundant override when the chosen keys equal the default; change the logic in
handleConflictReassign so it uses the same normalization as the recorder: if
pendingConflict.keys equals HOTKEYS[pendingConflict.targetId].key call
setOverride(pendingConflict.targetId, null), otherwise call
setOverride(pendingConflict.targetId, pendingConflict.keys); keep the existing
setOverride(pendingConflict.conflictId, null) and setPendingConflict(null)
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 54812c06-4fd9-462f-a7ec-6198efdbed05
📒 Files selected for processing (6)
apps/desktop/src/renderer/hotkeys/hooks/useBinding/useBinding.tsapps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.tsapps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.tsapps/desktop/src/renderer/hotkeys/utils/utils.tsapps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/keyboard/page.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/routes/_authenticated/layout.tsx
- apps/desktop/src/renderer/hotkeys/utils/utils.ts
- apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts
Comment on lines
+12
to
+31
| function captureHotkeyFromEvent(event: KeyboardEvent): string | null { | ||
| const key = event.key.toLowerCase(); | ||
| if (["shift", "ctrl", "alt", "meta", "dead", "unidentified"].includes(key)) | ||
| return null; | ||
|
|
||
| // Must include ctrl or meta (or be F1-F12) | ||
| const isFKey = /^f([1-9]|1[0-2])$/.test(key); | ||
| if (!isFKey && !event.ctrlKey && !event.metaKey) return null; | ||
|
|
||
| // Reject meta on non-Mac | ||
| if (PLATFORM !== "mac" && event.metaKey) return null; | ||
|
|
||
| const modifiers: string[] = []; | ||
| if (event.metaKey) modifiers.push("meta"); | ||
| if (event.ctrlKey) modifiers.push("ctrl"); | ||
| if (event.altKey) modifiers.push("alt"); | ||
| if (event.shiftKey) modifiers.push("shift"); | ||
|
|
||
| const ordered = MODIFIER_ORDER.filter((m) => modifiers.includes(m)); | ||
| return [...ordered, key].join("+"); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Chromium / Electron, what values does KeyboardEvent.key return for the Control key and the Space key?
💡 Result:
In Chromium and Electron, KeyboardEvent.key returns "Control" for the Control key and " " (space character) for the Space key.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values
- 2: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values
- 3: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key
- 4: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent.key
- 5: https://developer.mozilla.org/en-US/docs/web/api/ui_events/keyboard_event_key_values
- 6: https://w3.org/TR/uievents-key
🏁 Script executed:
cat -n apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts | head -70Repository: superset-sh/superset
Length of output: 2837
🏁 Script executed:
rg -n "const HOTKEYS|export.*HOTKEYS" --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg -n "const HOTKEYS|export.*HOTKEYS" -t tsRepository: superset-sh/superset
Length of output: 451
🏁 Script executed:
sed -n '571,600p' apps/desktop/src/renderer/hotkeys/registry.tsRepository: superset-sh/superset
Length of output: 251
🏁 Script executed:
sed -n '29,150p' apps/desktop/src/renderer/hotkeys/registry.ts | head -80Repository: superset-sh/superset
Length of output: 2417
🏁 Script executed:
rg -n "space\|meta\+space" apps/desktop/src/renderer/hotkeys/registry.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg -n "captureHotkeyFromEvent\|checkReserved" apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
cat -n apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts | tail -50Repository: superset-sh/superset
Length of output: 1769
Normalize event.key to handle Chromium/Electron key names before building shortcut strings.
Chromium/Electron reports the Ctrl key as "Control" (uppercase) and Space as a literal " ". The current code only applies toLowerCase(), which produces invalid shortcuts like "ctrl+control" when pressing Ctrl+K, and prevents Space-based shortcuts from matching reserved keys like "meta+space".
🔧 Add key aliases to normalize "Control" → "ctrl" and " " → "space"
+const KEY_ALIASES: Record<string, string> = {
+ control: "ctrl",
+ " ": "space",
+};
+
function captureHotkeyFromEvent(event: KeyboardEvent): string | null {
- const key = event.key.toLowerCase();
+ const key = KEY_ALIASES[event.key.toLowerCase()] ?? event.key.toLowerCase();
if (["shift", "ctrl", "alt", "meta", "dead", "unidentified"].includes(key))
return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function captureHotkeyFromEvent(event: KeyboardEvent): string | null { | |
| const key = event.key.toLowerCase(); | |
| if (["shift", "ctrl", "alt", "meta", "dead", "unidentified"].includes(key)) | |
| return null; | |
| // Must include ctrl or meta (or be F1-F12) | |
| const isFKey = /^f([1-9]|1[0-2])$/.test(key); | |
| if (!isFKey && !event.ctrlKey && !event.metaKey) return null; | |
| // Reject meta on non-Mac | |
| if (PLATFORM !== "mac" && event.metaKey) return null; | |
| const modifiers: string[] = []; | |
| if (event.metaKey) modifiers.push("meta"); | |
| if (event.ctrlKey) modifiers.push("ctrl"); | |
| if (event.altKey) modifiers.push("alt"); | |
| if (event.shiftKey) modifiers.push("shift"); | |
| const ordered = MODIFIER_ORDER.filter((m) => modifiers.includes(m)); | |
| return [...ordered, key].join("+"); | |
| const KEY_ALIASES: Record<string, string> = { | |
| control: "ctrl", | |
| " ": "space", | |
| }; | |
| function captureHotkeyFromEvent(event: KeyboardEvent): string | null { | |
| const key = KEY_ALIASES[event.key.toLowerCase()] ?? event.key.toLowerCase(); | |
| if (["shift", "ctrl", "alt", "meta", "dead", "unidentified"].includes(key)) | |
| return null; | |
| // Must include ctrl or meta (or be F1-F12) | |
| const isFKey = /^f([1-9]|1[0-2])$/.test(key); | |
| if (!isFKey && !event.ctrlKey && !event.metaKey) return null; | |
| // Reject meta on non-Mac | |
| if (PLATFORM !== "mac" && event.metaKey) return null; | |
| const modifiers: string[] = []; | |
| if (event.metaKey) modifiers.push("meta"); | |
| if (event.ctrlKey) modifiers.push("ctrl"); | |
| if (event.altKey) modifiers.push("alt"); | |
| if (event.shiftKey) modifiers.push("shift"); | |
| const ordered = MODIFIER_ORDER.filter((m) => modifiers.includes(m)); | |
| return [...ordered, key].join("+"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.ts`
around lines 12 - 31, In captureHotkeyFromEvent, normalize event.key before
building the shortcut string: after const key = event.key.toLowerCase(); map
known aliases (e.g. map "control" -> "ctrl" and the literal " " -> "space")
using a small keyAliases lookup and replace key with the normalized value; then
proceed with the existing exclusion list, F-key test, modifier checks, and final
join so you avoid producing duplicates like "ctrl+control" and so space-based
shortcuts become "space" (and thus match reserved combos like "meta+space");
reference function captureHotkeyFromEvent and constants MODIFIER_ORDER and
PLATFORM when making the change.
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 5, 2026
…erset-sh#3178) upstream 1219200 の取り込み: - shared/hotkeys.ts (1042行) + stores/hotkeys/ (357行) を削除 - renderer/hotkeys/ に新システム導入 (react-hotkeys-hook ベース) - useAppHotkey → useHotkey、HotkeyTooltipContent → HotkeyLabel に全面移行 - tRPC hotkeysルーター削除、メニューacceleratorハードコード化 - キーボード設定ページを新システムで書き直し フォーク固有の対応: - BROWSER_RELOAD / BROWSER_HARD_RELOAD を新レジストリに追加 - SEARCH_IN_FILES を新レジストリに追加 - HotkeyCategory に "Browser" カテゴリ追加 - menu.ts のフォーク独自メニュー項目 (Browser Reload) 維持 - browser-shortcut-events.ts 維持 - workspace/page.tsx のコンフリクト解消 (閉じ括弧修正含む)
6 tasks
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 5, 2026
refactor(desktop): ホットキーシステムをreact-hotkeys-hookに全面移行 (superset-sh#3178)
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 5, 2026
cherry-pick + 手動統合で内容を取り込み済み: - 1219200 superset-sh#3178 — rewrite hotkey system with react-hotkeys-hook ✓ フォーク固有の追加対応: - BROWSER_RELOAD / BROWSER_HARD_RELOAD / SEARCH_IN_FILES を新レジストリに追加 - HotkeyCategory に "Browser" カテゴリ追加 - 全 useHotkey に enabled: isActive ガード追加 (keep-alive workspace 対策) - menu.ts accelerator を registry.ts と一致させる修正 これで upstream/main との behind = 0
github-actions Bot
added a commit
that referenced
this pull request
Apr 10, 2026
The hotkey rewrite in #3178 introduced a blanket catch-all that bubbles every Ctrl/Meta combo away from xterm, breaking standard readline shortcuts (Ctrl+R, Ctrl+L, Ctrl+A, etc.). The TERMINAL_RESERVED allowlist only contained ctrl+c/d/z/s/q/\, so readline combos were intercepted by the app layer instead of reaching the terminal. Add the 12 missing readline shortcuts to TERMINAL_RESERVED in both utils.ts and useRecordHotkeys.ts so they pass through to xterm. Closes #3333
3 tasks
darklow
added a commit
to darklow/superset
that referenced
this pull request
Apr 10, 2026
The hotkey rewrite in superset-sh#3178 bubbles all Ctrl combos away from xterm unless they're in TERMINAL_RESERVED, which only had 6 entries. This restores standard readline shortcuts (Ctrl+R, Ctrl+L, Ctrl+A, etc.).
Kitenite
added a commit
that referenced
this pull request
Apr 12, 2026
Two bugs in the post-rewrite hotkey migration (#3178): 1. The "already migrated" guard checked for the `hotkey-overrides` localStorage key, but Zustand's persist middleware doesn't write it until the store mutates. Users who never customized anything re-ran the migration (and re-hit the legacy tRPC endpoint) every launch. Replaced with a dedicated `hotkey-overrides-migrated` sentinel set in every terminal branch. 2. The new registry declares punctuation via code-based names (`bracketleft`, `comma`, `slash`, …) because react-hotkeys-hook matches on `KeyboardEvent.code`. The old store held the literal character ("["). The migration copied overrides verbatim, so anything bound to `[ ] , . / \ ; ' backtick - =` silently stopped working after upgrade. Added a token-level translation table. Adds migrate.test.ts covering both fixes plus the existing branches.
3 tasks
darklow
added a commit
to darklow/superset
that referenced
this pull request
Apr 19, 2026
…ll ctrl combos Instead of allowlisting individual terminal shortcuts in TERMINAL_RESERVED, flip the logic: keep all ctrl/meta combos in xterm by default and only bubble events that match a registered app hotkey (isAppHotkeyEvent). This restores the pre-superset-sh#3178 behavior where unregistered shortcuts like Ctrl+R, Ctrl+L, Ctrl+O, etc. stay in the terminal.
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
1 participant
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.
Summary
Rewrites the desktop app's hotkey system from scratch, replacing 1,400 lines of custom key parsing/matching/normalization with
react-hotkeys-hookas the listener layer.renderer/hotkeys/module: registry with explicit per-platform keys,useHotkeyhook wrapping react-hotkeys-hook, Zustand + localStorage for user overrides,HotkeyLabelcomponent, display formattingshared/hotkeys.ts(1,042 lines): custommatchesHotkeyEvent,parseHotkeyString,normalizeKey,deriveNonMacDefault,toElectronAccelerator, and 27 copy-paste numbered entriesstores/hotkeys/store.ts(357 lines): old Zustand store with tRPC persistence, multi-window sync, anduseAppHotkeyhook83 files changed, 1,421 insertions, 2,737 deletions (net -1,316 lines)
Test plan
Summary by cubic
Rewrote the desktop hotkey system to use
react-hotkeys-hook, replacing the custom parser and store. Shortcuts now work in inputs and terminal panes, display correctly across platforms, and user overrides persist in localStorage with a one-time migration on startup.Refactors
renderer/hotkeys/with a per-platform registry, hooks (useHotkey,useHotkeyDisplay,useRecordHotkeys),HotkeyLabel, display formatting, and terminal utils.useAppHotkey/useHotkeyTextwith new hooks; replacedHotkeyTooltipContentwithHotkeyLabel.slash,comma,bracketleft/right) with proper display mapping; patched tests fordocument/window.addEventListener.Bug Fixes
HotkeyLabelis rendered without an id.useHotkeyto avoid missed handlers.Written for commit 93bccc7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor