desktop: configurable keyboard shortcuts#538
Conversation
📝 WalkthroughWalkthroughAdds a full in-app hotkeys subsystem: shared hotkey utilities, renderer Zustand store and hooks, TRPC import/export endpoints, AppState persistence and events, dynamic menu accelerators, and a complete Keyboard Shortcuts UI with recording, import/export, and conflict handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Renderer
participant Store as Hotkeys Store
participant TRPC
participant FS as File System
participant Emitter as Hotkeys Emitter
participant Menu as Menu System
User->>Renderer: Click "Export" in settings
Renderer->>TRPC: hotkeys.export.mutate()
TRPC->>FS: showSaveDialog()
FS-->>TRPC: path / cancelled
alt path chosen
TRPC->>TRPC: createHotkeysExport(appState.hotkeysState)
TRPC->>FS: write JSON file
TRPC-->>Renderer: { status: "ok", path }
else cancelled
TRPC-->>Renderer: { status: "cancelled" }
end
User->>Renderer: Change/record hotkey
Renderer->>Store: local update (useHotkeysStore.set)
Store->>TRPC: trpcHotkeysStorage.set(...)
TRPC->>Emitter: emit hotkeys change (version, updatedAt)
Emitter-->>Menu: trigger menu rebuild
Menu->>Menu: getMenuAccelerator(id) → rebuild menu
Menu-->>User: updated accelerators visible
sequenceDiagram
participant User
participant Renderer
participant TRPC
participant FS as File System
participant Dialog as Import Dialog
participant Store as Hotkeys Store
User->>Renderer: Click "Import" in settings
Renderer->>TRPC: hotkeys.import.mutate()
TRPC->>FS: showOpenDialog()
FS-->>TRPC: selected file / cancelled
alt file selected
TRPC->>FS: read JSON
TRPC->>TRPC: validate via zod schema
TRPC->>TRPC: buildHotkeysStateFromExport + normalize
TRPC-->>Renderer: { state, summary, path }
Renderer->>Dialog: show import summary
alt user confirms
Dialog->>Store: replace hotkeys state
Store->>TRPC: persist via trpcHotkeysStorage.set
else user resolves conflicts
Dialog->>Store: apply resolved bindings
end
Store-->>Renderer: UI updates with new hotkeys
else cancelled
TRPC-->>Renderer: { status: "cancelled" }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (5)apps/desktop/**/*.{ts,tsx}📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/**/src/**/**/[A-Z]*.tsx📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/desktop/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/desktop/src/{shared/ipc-channels.ts,main/**/*ipcs.ts,renderer/**/*.tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (1)
26-28: Potential duplicate key issue.Using
key={key}could cause React key warnings if the display array contains duplicate key names. Consider using the index as part of the key:🔎 Proposed fix
- {shortcut.display.map((key) => ( - <Kbd key={key}>{key}</Kbd> + {shortcut.display.map((key, index) => ( + <Kbd key={`${key}-${index}`}>{key}</Kbd> ))}apps/desktop/src/main/lib/hotkeys-events.ts (1)
1-8: Consider adding typed event constants and type-safe emitter.The untyped
EventEmitterand lack of event name constants could lead to runtime errors from typos. Consider defining event names as constants and optionally using a typed emitter pattern:🔎 Proposed improvement
import { EventEmitter } from "node:events"; export interface HotkeysStateChangedEvent { version: number; updatedAt: string; } -export const hotkeysEmitter = new EventEmitter(); +export const HOTKEYS_EVENTS = { + STATE_CHANGED: "hotkeys:state-changed", +} as const; + +export const hotkeysEmitter = new EventEmitter<{ + [HOTKEYS_EVENTS.STATE_CHANGED]: [HotkeysStateChangedEvent]; +}>();apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (1)
111-170: Consider extracting workspace jump hotkeys into a loop.While the explicit registration for each
JUMP_TO_WORKSPACE_Nhotkey is correct and works, there's some repetition that could be reduced. This is a minor refactor suggestion for maintainability.🔎 Optional: Reduce repetition with a loop
// Example approach using useMemo + useEffect pattern or a custom hook const workspaceJumpIds = [ "JUMP_TO_WORKSPACE_1", "JUMP_TO_WORKSPACE_2", // ... etc ] as const; // Register in a loop within a single useEffect, or keep as-is for explicitnessThe current explicit approach is valid and has the advantage of being easily searchable. This is purely a style preference.
apps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsx (1)
48-54: Consider using index-based keys forKbdelements.Using
key={key}could cause issues if a hotkey display contains duplicate symbols (e.g., a theoretical"Shift+Shift"or platform edge cases). Using the array index is safer here since the display array is small and stable per render.🔎 Suggested fix
return ( <KbdGroup> - {display.map((key) => ( - <Kbd key={key}>{key}</Kbd> + {display.map((key, index) => ( + <Kbd key={index}>{key}</Kbd> ))} </KbdGroup> );apps/desktop/src/lib/trpc/routers/hotkeys/index.ts (2)
16-27: Schema validation is permissive—consider addingschemaVersionvalidation.The schema accepts any
schemaVersion: z.number()without validating it matches the current version. If the export format evolves, importing an incompatible version could silently produce unexpected state. Consider adding a version check after parsing:🔎 Suggested version validation
try { const raw = await readFile(filePath, "utf-8"); const parsed = hotkeysExportSchema.parse(JSON.parse(raw)); + + // Validate schema version compatibility + const CURRENT_SCHEMA_VERSION = 1; // or import from shared/hotkeys + if (parsed.schemaVersion !== CURRENT_SCHEMA_VERSION) { + return { + canceled: false, + error: `Incompatible file version: expected ${CURRENT_SCHEMA_VERSION}, got ${parsed.schemaVersion}`, + }; + } + const exportFile: HotkeysExportFile = {
94-103: Redundant object reconstruction after Zod parse.The
parsedobject from Zod is already validated. The manual reconstruction intoexportFilewith fallbacks (?? {}) is defensive but adds verbosity. Zod's output is type-safe—consider using it directly or adjusting the schema to require the hotkeys object.apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
315-347: Functional hotkey registration hook with one linting consideration.The hook correctly uses a ref to keep the callback stable while allowing updates. The environment guard handles SSR/non-browser contexts.
The
...depsspread in the dependency array (line 346) works but may trigger ESLint'sreact-hooks/exhaustive-depsrule. If this causes linting warnings, consider accepting deps as a ref or documenting the pattern.🔎 Alternative: deps as a serialized key
- deps: unknown[] = [], + deps?: React.DependencyList, ) { // ... useEffect(() => { // ... - }, [enabled, keys, preventDefault, ...deps]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [enabled, keys, preventDefault, ...(deps ?? [])]); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
apps/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/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/HotkeyTooltipContent/HotkeyTooltipContent.tsxapps/desktop/src/renderer/components/HotkeyTooltipContent/index.tsapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/CreateWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.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/TabPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/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.tsbunfig.toml
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/stores/hotkeys/index.tsapps/desktop/src/renderer/components/HotkeyTooltipContent/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/app-state/index.tsapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/lib/menu.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/CreateWorkspaceButton.tsxapps/desktop/src/shared/hotkeys.test.tsapps/desktop/src/main/lib/app-state/schemas.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsxapps/desktop/src/main/lib/hotkeys-events.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsxapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsxapps/desktop/src/lib/trpc/routers/hotkeys/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsxapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/lib/trpc/routers/ui-state/index.tsapps/desktop/src/shared/hotkeys.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type safety and avoid
anytypes unless absolutely necessary in TypeScript files
Files:
apps/desktop/src/renderer/stores/hotkeys/index.tsapps/desktop/src/renderer/components/HotkeyTooltipContent/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/app-state/index.tsapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/lib/menu.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/CreateWorkspaceButton.tsxapps/desktop/src/shared/hotkeys.test.tsapps/desktop/src/main/lib/app-state/schemas.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsxapps/desktop/src/main/lib/hotkeys-events.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsxapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsxapps/desktop/src/lib/trpc/routers/hotkeys/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsxapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/lib/trpc/routers/ui-state/index.tsapps/desktop/src/shared/hotkeys.ts
apps/desktop/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Move Node.js functionality needed in Electron renderer to
src/main/lib/and communicate via IPC
Files:
apps/desktop/src/renderer/stores/hotkeys/index.tsapps/desktop/src/renderer/components/HotkeyTooltipContent/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/app-state/index.tsapps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/lib/menu.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/CreateWorkspaceButton.tsxapps/desktop/src/shared/hotkeys.test.tsapps/desktop/src/main/lib/app-state/schemas.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsxapps/desktop/src/main/lib/hotkeys-events.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsxapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsxapps/desktop/src/lib/trpc/routers/hotkeys/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsxapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/lib/trpc/routers/ui-state/index.tsapps/desktop/src/shared/hotkeys.ts
apps/**/src/**/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Structure component folders with one component per file using format
ComponentName/ComponentName.tsxwithindex.tsbarrel export
Files:
apps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/CreateWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsxapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsx
apps/desktop/src/{shared/ipc-channels.ts,main/**/*ipcs.ts,renderer/**/*.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Define all Electron IPC channel types in
apps/desktop/src/shared/ipc-channels.tsbefore implementing handlers
Files:
apps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SidebarControl.tsxapps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/CreateWorkspaceButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsxapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate tests with their source files using the same folder structure (e.g.,
ComponentName.test.tsxnext toComponentName.tsx)
Files:
apps/desktop/src/shared/hotkeys.test.ts
🧠 Learnings (11)
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/**/*.{ts,tsx} : Move Node.js functionality needed in Electron renderer to `src/main/lib/` and communicate via IPC
Applied to files:
apps/desktop/src/renderer/stores/hotkeys/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/shared/hotkeys.test.tsapps/desktop/src/main/lib/hotkeys-events.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/stores/hotkeys/store.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/{shared/ipc-channels.ts,main/**/*ipcs.ts,renderer/**/*.tsx} : Define all Electron IPC channel types in `apps/desktop/src/shared/ipc-channels.ts` before implementing handlers
Applied to files:
apps/desktop/src/renderer/stores/hotkeys/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/lib/menu.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsxapps/desktop/src/shared/hotkeys.test.tsapps/desktop/src/main/lib/hotkeys-events.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/lib/trpc/routers/hotkeys/index.tsapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/lib/trpc/routers/ui-state/index.tsapps/desktop/src/shared/hotkeys.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in `src/main/index.ts` with `override: true` before any other imports in the desktop app
Applied to files:
apps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/windows/main.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/{renderer,lib/electron-router-dom.ts} : Never import Node.js modules in Electron renderer or shared code (`src/renderer/` and `src/lib/electron-router-dom.ts`)
Applied to files:
apps/desktop/src/renderer/stores/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/lib/trpc/routers/hotkeys/index.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/**/src/**/**/[A-Z]*.tsx : Structure component folders with one component per file using format `ComponentName/ComponentName.tsx` with `index.ts` barrel export
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to src/components/{ui,ai-elements,react-flow}/**/*.tsx : shadcn/ui components in `src/components/ui/`, `src/components/ai-elements`, and `src/components/react-flow/` use kebab-case single files instead of the folder structure convention
Applied to files:
apps/desktop/src/renderer/screens/main/index.tsx
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to **/*.test.{ts,tsx} : Co-locate tests with their source files using the same folder structure (e.g., `ComponentName.test.tsx` next to `ComponentName.tsx`)
Applied to files:
apps/desktop/src/shared/hotkeys.test.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/stores/hotkeys/store.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/lib/trpc/routers/ui-state/index.ts
🧬 Code graph analysis (25)
apps/desktop/src/main/lib/app-state/index.ts (1)
apps/desktop/src/main/lib/app-state/schemas.ts (1)
defaultAppState(22-35)
apps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsx (2)
apps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsx (1)
HotkeyTooltipContent(26-88)apps/desktop/src/renderer/components/HotkeyTooltipContent/index.ts (1)
HotkeyTooltipContent(2-2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx (2)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useHotkeyText(200-209)packages/ui/src/components/ui/context-menu.tsx (1)
ContextMenuShortcut(245-245)
apps/desktop/src/renderer/screens/main/components/TopBar/SidebarControl.tsx (2)
apps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsx (1)
HotkeyTooltipContent(26-88)apps/desktop/src/renderer/components/HotkeyTooltipContent/index.ts (1)
HotkeyTooltipContent(2-2)
apps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx (1)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useHotkeyDisplay(194-198)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
apps/desktop/src/shared/hotkeys.ts (2)
isTerminalReservedEvent(291-296)matchesHotkeyEvent(221-254)apps/desktop/src/renderer/stores/hotkeys/store.ts (2)
getHotkeyKeys(188-192)isAppHotkeyEvent(243-252)
apps/desktop/src/main/lib/menu.ts (3)
apps/desktop/src/shared/hotkeys.ts (4)
HotkeyId(30-30)getCurrentPlatform(169-173)getEffectiveHotkey(563-570)toElectronAccelerator(690-718)apps/desktop/src/main/lib/app-state/index.ts (1)
appState(47-59)apps/desktop/src/main/lib/hotkeys-events.ts (1)
hotkeysEmitter(8-8)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (1)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useAppHotkey(315-347)
apps/desktop/src/renderer/screens/main/index.tsx (2)
apps/desktop/src/renderer/stores/hotkeys/store.ts (2)
useHotkeysSync(284-306)useAppHotkey(315-347)apps/desktop/src/renderer/stores/tabs/pane-refs.ts (1)
getPaneDimensions(13-20)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx (2)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useAppHotkey(315-347)apps/desktop/src/renderer/stores/tabs/utils.ts (2)
getPreviousPaneId(248-260)getNextPaneId(230-242)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useAppHotkey(315-347)
apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/CreateWorkspaceButton.tsx (3)
apps/desktop/src/renderer/stores/hotkeys/store.ts (2)
useAppHotkey(315-347)useHotkeyText(200-209)apps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsx (1)
HotkeyTooltipContent(26-88)packages/ui/src/components/ui/dropdown-menu.tsx (1)
DropdownMenuShortcut(260-260)
apps/desktop/src/shared/hotkeys.test.ts (1)
apps/desktop/src/shared/hotkeys.ts (6)
canonicalizeHotkey(175-179)canonicalizeHotkeyForPlatform(181-189)deriveNonMacDefault(307-324)hotkeyFromKeyboardEvent(256-283)toElectronAccelerator(690-718)isTerminalReservedEvent(291-296)
apps/desktop/src/main/lib/app-state/schemas.ts (1)
apps/desktop/src/shared/hotkeys.ts (2)
HotkeysState(34-37)createDefaultHotkeysState(625-634)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/EmptyTabView.tsx (1)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useHotkeyDisplay(194-198)
apps/desktop/src/lib/trpc/routers/index.ts (1)
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts (1)
createHotkeysRouter(45-127)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx (2)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useHotkeyText(200-209)packages/ui/src/components/ui/dropdown-menu.tsx (1)
DropdownMenuShortcut(260-260)
apps/desktop/src/main/windows/main.ts (1)
apps/desktop/src/main/lib/menu.ts (1)
registerMenuHotkeyUpdates(30-36)
apps/desktop/src/renderer/lib/trpc-storage.ts (2)
apps/desktop/src/renderer/lib/trpc-client.ts (1)
trpcClient(9-11)apps/desktop/src/shared/hotkeys.ts (1)
HotkeysState(34-37)
apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx (1)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useHotkeyText(200-209)
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts (3)
apps/desktop/src/shared/hotkeys.ts (7)
HotkeysState(34-37)HotkeysExportFile(39-44)createHotkeysExport(636-649)buildHotkeysStateFromExport(651-671)getCurrentPlatform(169-173)normalizeBindingsWithDefaults(604-623)getHotkeysSummary(673-688)apps/desktop/src/lib/trpc/index.ts (1)
router(47-47)apps/desktop/src/main/lib/app-state/index.ts (1)
appState(47-59)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsx (1)
apps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsx (1)
HotkeyTooltipContent(26-88)
apps/desktop/src/renderer/stores/hotkeys/store.ts (2)
apps/desktop/src/shared/hotkeys.ts (11)
HotkeysState(34-37)HotkeyPlatform(8-8)HotkeyId(30-30)canonicalizeHotkeyForPlatform(181-189)HotkeyCategory(10-15)HotkeyDefinition(17-28)HOTKEYS(350-529)matchesHotkeyEvent(221-254)isTerminalReservedHotkey(285-289)isOsReservedHotkey(298-305)hotkeyFromKeyboardEvent(256-283)apps/desktop/src/renderer/lib/trpc-storage.ts (1)
trpcHotkeysStorage(67-78)
apps/desktop/src/lib/trpc/routers/ui-state/index.ts (4)
apps/desktop/src/lib/trpc/index.ts (1)
router(47-47)apps/desktop/src/shared/hotkeys.ts (3)
HotkeysState(34-37)HOTKEYS_STATE_VERSION(46-46)buildOverridesFromBindings(583-602)apps/desktop/src/main/lib/app-state/index.ts (1)
appState(47-59)apps/desktop/src/main/lib/hotkeys-events.ts (1)
hotkeysEmitter(8-8)
apps/desktop/src/shared/hotkeys.ts (1)
apps/desktop/src/shared/constants.ts (1)
PLATFORM(4-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (47)
bunfig.toml (1)
1-2: Configuration is correct and necessary for the monorepo to function.The
isolatedlinker prevents hoisting that would incorrectly resolvepath-scurry@2(which requireslru-cache@11) with the olderlru-cache@6that other dependencies in the tree need, causing missingLRUCacheerrors during the@superset/desktopnative rebuild. This is not a temporary workaround but a required configuration given the dependency graph. Bun 1.3.0 fully supports this option.apps/desktop/src/main/windows/main.ts (1)
56-57: LGTM! Clean integration of dynamic menu hotkey updates.The placement of
registerMenuHotkeyUpdates()aftercreateApplicationMenu()correctly ensures the menu is created first, then listens for hotkey changes. The function is idempotent (per the context frommenu.ts), preventing duplicate listeners on window reopens.apps/desktop/src/shared/hotkeys.ts (1)
1-718: LGTM! Comprehensive and well-designed hotkey management system.This module provides a robust, platform-aware hotkey system with excellent abstractions:
- Clear separation of concerns (parsing, canonicalization, formatting, validation)
- Type-safe interfaces and constants
- Platform-specific defaults with automatic derivation for non-Mac platforms
- Reserved hotkey detection (terminal and OS-level)
- Electron accelerator conversion
- State management with export/import support
- The
defineHotkeyfactory pattern cleanly handles platform defaultsThe implementation is thorough and handles edge cases well (slash key, arrow keys, modifier-only events, etc.).
apps/desktop/src/renderer/stores/index.ts (1)
2-2: LGTM! Standard barrel export.The addition of the hotkeys store export follows the existing pattern and properly exposes the hotkeys API to consumers.
apps/desktop/src/renderer/stores/hotkeys/index.ts (1)
1-1: LGTM! Clean barrel export.Standard re-export pattern that provides a clean import surface for the hotkeys store.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabContentContextMenu.tsx (1)
21-21: LGTM! Clean dynamic hotkey integration.The component correctly replaces the hardcoded shortcut with dynamic hotkey text:
- Uses
useHotkeyTextto fetch the current binding- Conditionally renders the shortcut only when assigned
- Follows React best practices and matches patterns used elsewhere in the codebase
Also applies to: 49-50, 67-69
apps/desktop/src/renderer/screens/main/components/AvatarDropdown/AvatarDropdown.tsx (1)
28-28: LGTM! Successful migration to dynamic hotkeys.The component correctly migrates from static
HOTKEYSconstants to the dynamicuseHotkeyDisplayhook, enabling runtime hotkey customization. The implementation maintains the same rendering logic while supporting the new configurable hotkey system.Also applies to: 37-37, 121-123
apps/desktop/src/main/lib/app-state/index.ts (1)
25-32: LGTM! Proper deep merge for nested hotkeys state.The
hotkeysStatemerge correctly handles the nestedbyPlatformstructure with a two-level merge:
- Shallow merge of top-level
hotkeysStatefields (version)- Deep merge of the nested
byPlatformobject to ensure all platforms existThis gracefully handles legacy/partial data and follows the same pattern as
tabsStateandthemeState.apps/desktop/src/renderer/lib/trpc-storage.ts (1)
64-78: The wrapping pattern intrpcHotkeysStorageis correct and necessary. The hotkeys store's persist configuration explicitly usespartialize: (state) => ({ hotkeysState: state.hotkeysState }), which means Zustand passes a{ hotkeysState: ... }structure to the storage adapter'ssetmethod. The storage adapter correctly handles this by returning the same structure fromgetand unwrapping it before sending to the API. This is intentional design, not an inconsistency with the other adapters.apps/desktop/src/renderer/screens/main/components/SettingsButton/SettingsButton.tsx (1)
24-24: Verify hotkeyId matches the button's intended action.The button is labeled "Open settings" but uses
hotkeyId="SHOW_HOTKEYS". If theSHOW_HOTKEYShotkey opens settings to the keyboard shortcuts section specifically, this is fine. Otherwise, consider using anOPEN_SETTINGShotkeyId or similar to match the button's purpose.apps/desktop/src/main/lib/app-state/schemas.ts (1)
4-4: LGTM!Clean integration of
hotkeysStateinto the app state schema following the established patterns fortabsStateandthemeState. UsingcreateDefaultHotkeysState()factory function ensures consistent initialization.Also applies to: 19-19, 34-34
apps/desktop/src/lib/trpc/routers/index.ts (1)
9-9: LGTM!The hotkeys router is correctly integrated following the established pattern for routers requiring window access. As per coding guidelines, tRPC is properly used for Electron interprocess communication.
Also applies to: 37-37
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/TabPane.tsx (1)
154-157: LGTM!Clean integration of
HotkeyTooltipContentfor both split and close pane actions. The hotkeyIds appropriately match the button actions, and the refactor aligns with the broader tooltip standardization effort.Also applies to: 171-174
apps/desktop/src/renderer/screens/main/components/TopBar/SidebarControl.tsx (1)
28-31: LGTM!Clean refactor to use
HotkeyTooltipContentwith semantically correcthotkeyId. The label and hotkeyId properly align with the button's toggle sidebar action.apps/desktop/src/renderer/components/HotkeyTooltipContent/index.ts (1)
1-2: LGTM!Standard barrel export following the component folder structure guidelines. Correctly separates type and value exports.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (3)
11-13: LGTM! Clean migration to configurable hotkey utilities.The imports correctly pull in the necessary utilities from the centralized hotkey stores -
getHotkeyKeysandisAppHotkeyEventfrom the renderer store, andisTerminalReservedEventandmatchesHotkeyEventfrom the shared module.
254-258: LGTM! Proper null-safety on configurable clear shortcut.The logic correctly handles the case where
CLEAR_TERMINALmight be unassigned (getHotkeyKeysreturnsnull), ensuringmatchesHotkeyEventis only called when a valid key binding exists.
270-287: LGTM! Correctly forwards app hotkeys to document for react-hotkeys-hook.The
isAppHotkeyEventcheck now dynamically determines if the event matches any configured app hotkey, rather than relying on hardcoded key combinations. This ensures terminal keyboard events for app-level shortcuts are properly re-dispatched.apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx (2)
67-70: LGTM! Dynamic hotkey text retrieval with proper visibility checks.The pattern of comparing
useHotkeyTextresult against"Unassigned"to determine visibility is consistent with the broader PR approach. This ensures shortcuts are only displayed when they have valid bindings.
114-116: LGTM! Graceful fallback for unassigned shortcuts in tooltip.Using
"—"as a placeholder when the shortcut is unassigned provides a clear visual indication that no shortcut is configured, rather than leaving an empty space.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
181-188: LGTM! Correctly scoped hotkey registration.The migration to
useAppHotkeyproperly:
- Uses the
FIND_IN_TERMINALidentifier for configurable binding- Conditionally enables the hotkey only when
isFocusedis true- Includes
isFocusedin the dependency array to re-register when focus changes- Sets
preventDefault: trueto prevent browser default behaviorapps/desktop/src/renderer/screens/main/index.tsx (3)
73-73: LGTM! Global hotkeys synchronization hook placement.Calling
useHotkeysSync()at the top level of the main screen ensures hotkey state changes from the backend are synchronized across the app, enabling live updates when users modify shortcuts in settings.
91-102: LGTM! Properly migrated to useAppHotkey with guard conditions.Both
SHOW_HOTKEYSandTOGGLE_SIDEBARhotkeys are correctly wired with appropriate dependency arrays. TheisWorkspaceViewguard ensures sidebar toggle only works in the workspace context.
123-196: LGTM! Split operations correctly migrated with complete dependency arrays.The
SPLIT_AUTO,SPLIT_RIGHT, andSPLIT_DOWNhotkeys maintain the same logic as before with proper guard conditions (isWorkspaceView,activeTabId,focusedPaneId,activeTab). All required dependencies are included in the arrays.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/index.tsx (1)
81-89: LGTM! Simplified workspace switch handler.The
handleWorkspaceSwitchfunction now takes an index directly instead of parsing from keyboard events, making the logic cleaner and delegating key detection to theuseAppHotkeyinfrastructure.apps/desktop/src/lib/trpc/routers/ui-state/index.ts (3)
166-173: LGTM! Hotkeys state schema correctly validates input.The schema properly defines the expected structure with version number and per-platform override records. Using
.default({})ensures missing platform entries are initialized correctly.
251-261: LGTM! Subscription follows the required observable pattern.As per the coding guidelines for tRPC subscriptions with trpc-electron, this correctly uses the
observablepattern from@trpc/server/observableinstead of async generators. The listener cleanup in the return function prevents memory leaks.
216-249: LGTM! Set procedure properly normalizes and persists hotkey state.The implementation:
- Validates version against
HOTKEYS_STATE_VERSION- Normalizes each platform's bindings via
buildOverridesFromBindings- Persists to disk with
appState.write()- Emits change event for subscribers
This ensures consistent state across the app.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx (3)
43-64: LGTM! Tab and pane management hotkeys correctly migrated.
NEW_TERMINALandCLOSE_TERMINALhotkeys maintain proper guard conditions and include all necessary dependencies in their arrays.
67-118: LGTM! Navigation hotkeys properly implemented.The terminal and pane navigation hotkeys (
PREV_TERMINAL,NEXT_TERMINAL,PREV_PANE,NEXT_PANE) correctly use the layout utilities and include complete dependency arrays withactiveTab?.layout.
124-149: LGTM! External action hotkeys correctly wired.
OPEN_IN_APPandCOPY_PATHhotkeys properly guard against missingactiveWorkspace?.worktreePathand include all mutation-related dependencies.apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceTabs/CreateWorkspaceButton.tsx (3)
117-125: LGTM! Hotkey registrations correctly wired.All three workspace creation hotkeys (
NEW_WORKSPACE,QUICK_CREATE_WORKSPACE,OPEN_PROJECT) are registered with their corresponding callback handlers and proper dependency arrays.
150-160: LGTM! HotkeyTooltipContent provides clear multi-action hints.The tooltip now displays all three available actions with their respective shortcuts using the
itemsprop pattern, giving users a quick overview of available keyboard shortcuts.
174-205: LGTM! Conditional shortcut display in dropdown menu.Each dropdown item correctly renders its shortcut only when assigned, using the
showXxxShortcutflags derived from theuseHotkeyTextcomparisons against"Unassigned".apps/desktop/src/shared/hotkeys.test.ts (1)
1-74: LGTM! Good foundational test coverage for hotkey utilities.The tests appropriately cover critical paths: modifier normalization, platform rejection, key derivation, keyboard event capture, Electron accelerator conversion, and terminal-reserved detection. Co-location with
hotkeys.tsfollows project conventions.Consider expanding coverage in a follow-up for edge cases like:
deriveNonMacDefaultwith non-meta keys (the unchanged path)isTerminalReservedEventwith non-reserved combinations (negative case)hotkeyFromKeyboardEventreturningnullfor modifier-only pressesapps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx (2)
94-98: LGTM! Clean integration of dynamic hotkey display.The approach of checking for
"Unassigned"string is consistent withuseHotkeyTextbehavior which returns"Unassigned"when no hotkey is bound. The derivedshow*flags cleanly gate the shortcut display throughout the component.
181-185: Consistent shortcut indicator pattern across menu items.The shortcut display correctly appears only for the last-used app item, providing clear context for which action the hotkey triggers. This pattern is consistently applied in both main menu and JetBrains submenu.
apps/desktop/src/main/lib/menu.ts (2)
30-36: Clean listener registration pattern.The boolean flag prevents duplicate subscriptions effectively. Rebuilding the entire menu on hotkey change is the standard Electron approach since accelerators are baked into the menu template. For infrequent user-initiated hotkey changes, this is appropriate.
22-28:appStateinitialization timing is correctly ordered.The implementation properly ensures
initAppState()is awaited (line 220 inindex.ts) beforeMainWindow()is instantiated (line 230), which in turn callscreateApplicationMenu(). By the timegetMenuAccelerator()accessesappState.data.hotkeysState, initialization is complete. No changes needed.apps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsx (1)
26-87: Well-designed flexible component API.The component elegantly handles both single hotkey and multi-item tooltip patterns. The
showUnassignedflag with sensible default provides good UX control. The filtering logic correctly excludes unassigned items when the flag is false.apps/desktop/src/renderer/screens/main/components/SettingsView/KeyboardShortcutsSettings.tsx (4)
133-179: Good implementation of keyboard recording with proper cleanup.The recording effect correctly:
- Uses capture phase to intercept before other handlers
- Handles Escape to cancel, Backspace/Delete to unassign
- Checks terminal and OS reserved hotkeys with user feedback
- Cleans up the listener on unmount or when recording stops
One consideration: the effect depends on
setHotkeywhich is a stable Zustand action, so no stale closure risk there.
41-88: Clean HotkeyRow component design.The component correctly separates presentation from state management. Using
useHotkeyDisplayensures the row updates reactively when hotkeys change. The recording state visual feedback is clear.
185-225: Solid import/export flow with proper error handling.Both handlers correctly check discriminated union cases (
canceled,error) before proceeding. Toast notifications provide clear user feedback. The async flow properly awaits mutations.
326-369: Well-structured conflict resolution dialog.The dialog provides clear context about the conflict and offers Cancel/Reassign actions. The
AlertDialogDescriptioncorrectly usesasChildwith a div for complex content. ThehandleConflictReassignfunction properly batches the hotkey updates.apps/desktop/src/renderer/stores/hotkeys/store.ts (3)
64-176: Well-architected Zustand store with proper persistence.The store correctly:
- Uses
devtoolsandpersistmiddleware- Partializes state to only persist
hotkeysState- Implements immutable update patterns in all actions
- Handles canonicalization and default comparison to minimize override storage
Following project guidelines by using Zustand for state management.
284-306: Sync hook correctly bridges tRPC subscriptions to local state.The pattern of fetching on subscription notification and comparing before replacing is appropriate—it avoids unnecessary re-renders while keeping state synchronized across processes. The error logging provides visibility for debugging.
220-282: Comprehensive utility functions with correct typing.The utilities provide clean abstractions:
useHotkeysByCategoryproperly memoizes and respectsisHiddenisAppHotkeyEventuses synchronousgetState()appropriately for event handlersgetHotkeyConflictcorrectly excludes the current hotkey when checking for conflictsGood use of TypeScript's discriminated return type in
isReservedHotkey.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts (1)
16-27: Consider exporting the schema for consistency.The AI summary indicates
hotkeysExportSchemashould be exported, but it's currently only used internally. If there's no need for external validation (e.g., in tests or other modules), the current implementation is fine.💡 Proposed export if needed elsewhere
-const hotkeysExportSchema = z.object({ +export const hotkeysExportSchema = z.object({
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type safety and avoid
anytypes unless absolutely necessary in TypeScript files
Files:
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts
apps/desktop/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Move Node.js functionality needed in Electron renderer to
src/main/lib/and communicate via IPC
Files:
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts
🧠 Learnings (3)
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/{shared/ipc-channels.ts,main/**/*ipcs.ts,renderer/**/*.tsx} : Define all Electron IPC channel types in `apps/desktop/src/shared/ipc-channels.ts` before implementing handlers
Applied to files:
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from `trpc/server/observable` instead of async generators, as the library explicitly checks `isObservable(result)` and throws an error otherwise
Applied to files:
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts
🧬 Code graph analysis (1)
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts (2)
apps/desktop/src/shared/hotkeys.ts (7)
HotkeysState(34-37)HotkeysExportFile(39-44)createHotkeysExport(636-649)buildHotkeysStateFromExport(651-671)getCurrentPlatform(169-173)normalizeBindingsWithDefaults(604-623)getHotkeysSummary(673-688)apps/desktop/src/lib/trpc/index.ts (1)
router(47-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (2)
apps/desktop/src/lib/trpc/routers/hotkeys/index.ts (2)
63-76: LGTM - Error handling properly implemented.The try/catch block correctly handles I/O errors from
writeFile, addressing the previous review feedback. Error messages are extracted defensively and returned in a consistent format.
100-109: Defensive reconstruction ensures platform keys exist.The manual reconstruction of
HotkeysExportFileafter schema validation adds default empty objects for missing platform data. While slightly redundant, this defensive approach ensures downstream code can safely access all platform keys without additional null checks.
- P1: Enforce ctrl/meta modifier requirement for hotkeys at capture and import time, preventing alt-only/shift-only shortcuts that wouldn't work in terminal - P2: Add skip-persist flag to prevent echo writes during hotkeys sync - P2: Extend test mock to include uiState.hotkeys stubs
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/shared/hotkeys.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/stores/hotkeys/store.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/shared/hotkeys.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type safety and avoid
anytypes unless absolutely necessary in TypeScript files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/shared/hotkeys.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate tests with their source files using the same folder structure (e.g.,
ComponentName.test.tsxnext toComponentName.tsx)
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts
apps/desktop/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Move Node.js functionality needed in Electron renderer to
src/main/lib/and communicate via IPC
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/renderer/lib/trpc-storage.tsapps/desktop/src/shared/hotkeys.ts
🧠 Learnings (3)
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/renderer/lib/trpc-storage.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/lib/trpc-storage.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/{shared/ipc-channels.ts,main/**/*ipcs.ts,renderer/**/*.tsx} : Define all Electron IPC channel types in `apps/desktop/src/shared/ipc-channels.ts` before implementing handlers
Applied to files:
apps/desktop/src/shared/hotkeys.ts
🧬 Code graph analysis (2)
apps/desktop/src/renderer/lib/trpc-storage.ts (2)
apps/desktop/src/renderer/lib/trpc-client.ts (1)
trpcClient(9-11)apps/desktop/src/shared/hotkeys.ts (1)
HotkeysState(34-37)
apps/desktop/src/shared/hotkeys.ts (1)
apps/desktop/src/shared/constants.ts (1)
PLATFORM(4-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (10)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts (1)
22-27: Remove the unnecessaryuiState.hotkeysmock—these functions don't use trpcClient.The tested functions (
getDefaultTerminalThemeandgetDefaultTerminalBg) only access localStorage and never referencetrpcClient.uiState.hotkeys. The mock at lines 22–27 is not required for the test suite and can be removed. Even theexternalmocks above it are unused by the tested functions.Likely an incorrect or invalid review comment.
apps/desktop/src/shared/hotkeys.ts (7)
8-46: Well-structured type definitions.The type system for the hotkeys framework is comprehensive and self-documenting. The use of
HotkeyPlatformunion type,HotkeyCategorydiscriminator, and versioned state structure provides good type safety and extensibility.
133-137: Good edge case handling for space character.The special logic at Line 135 correctly preserves the raw space character when
trimmedbecomes empty, preventing spaces from being lost during normalization.
273-276: Correctly enforces primary modifier requirement.This check ensures app hotkeys include Ctrl or Meta, preventing conflicts with terminal input. Aligns with the PR objective to avoid alt-only/shift-only shortcuts that fail in terminals.
612-615: Primary modifier enforcement correctly applied.The
hasPrimaryModifiercheck ensures imported/configured hotkeys include Ctrl or Meta, preventing shortcuts that would conflict with terminal input. This correctly implements the PR objective to validate hotkey quality.
709-737: Electron accelerator conversion is well-implemented.The conversion correctly maps modifiers and keys to Electron's accelerator format. The platform check at Line 716 appropriately prevents meta-based shortcuts on non-Darwin platforms.
365-544: Comprehensive and well-typed hotkey definitions.The consistent use of
defineHotkeyand theas const satisfies Record<string, HotkeyDefinition>pattern at Line 544 provides excellent type safety. The hidden hotkeys (e.g., NEW_WINDOW at Line 515) are appropriate for reserved future functionality.
546-707: Well-designed state management API.The functions provide a clean separation between defaults, overrides, and effective bindings. The export/import asymmetry is intentional and correct:
createHotkeysExportexports full bindings (Lines 663-665) whilebuildHotkeysStateFromExportimports only overrides (Lines 676-687), minimizing stored data.apps/desktop/src/renderer/lib/trpc-storage.ts (2)
5-13: Pragmatic skip-persist mechanism for sync coordination.The module-level
skipNextHotkeysPersistflag effectively prevents echo writes during remote synchronization. While module-level mutable state is typically avoided, it's acceptable here since there should be a single hotkeys store instance per renderer process.
74-93: Consistent storage adapter implementation.The
trpcHotkeysStorageadapter follows the established pattern fromtrpcTabsStorageandtrpcThemeStorage. The wrapping/unwrapping ofhotkeysState(Lines 81, 89) maintains symmetry between get and set operations. The skip-persist check (Lines 85-88) correctly prevents echo writes during synchronization.
- Fix import ordering in hotkeys store.ts (biome organizeImports) - Resolve merge conflict in Terminal/helpers.ts (combined JSDoc comments) - Add comprehensive test coverage for deriveNonMacDefault function - null input handling - invalid hotkey handling - unchanged hotkey when no meta modifier - simple meta to ctrl+shift mapping - meta+alt to ctrl+alt+shift mapping
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/desktop/src/renderer/stores/hotkeys/store.ts (3)
243-249: Unstable dependency inuseMemo.The dependency
options?.includeHiddenis correct for the primitive boolean value, but if a caller passes a new object literal on each render (e.g.,useHotkeysByCategory({ includeHidden: false })), theoptionsreference changes whileoptions?.includeHiddenremains stable. Consider acceptingincludeHiddenas a direct boolean parameter to make the API clearer and avoid potential confusion.🔎 Proposed simplification
-export function useHotkeysByCategory(options?: { - includeHidden?: boolean; -}): Record<HotkeyCategory, Array<HotkeyDefinition & { id: HotkeyId }>> { +export function useHotkeysByCategory( + includeHidden = false, +): Record<HotkeyCategory, Array<HotkeyDefinition & { id: HotkeyId }>> { return useMemo(() => { const grouped: Record< HotkeyCategory, Array<HotkeyDefinition & { id: HotkeyId }> > = { Workspace: [], Layout: [], Terminal: [], Window: [], Help: [], }; for (const [id, hotkey] of Object.entries(HOTKEYS)) { - if (!options?.includeHidden && hotkey.isHidden) continue; + if (!includeHidden && hotkey.isHidden) continue; grouped[hotkey.category].push({ id: id as HotkeyId, ...hotkey }); } return grouped; - }, [options?.includeHidden]); + }, [includeHidden]); }
300-310: Shallow key sorting doesn't produce order-independent comparison for nested objects.
JSON.stringify(state, Object.keys(state).sort())uses the second argument as a "replacer array" which filters properties but doesn't recursively sort keys in nested objects likebyPlatform. If platform sub-objects have keys in different orders, this comparison could yield false negatives (treating identical states as different).Consider using a deep equality utility or a recursive key-sorting approach:
🔎 Proposed fix using deep equality
+import { isEqual } from "lodash"; + trpc.uiState.hotkeys.subscribe.useSubscription(undefined, { onData: () => { trpcClient.uiState.hotkeys.get .query() .then((state: HotkeysState) => { const current = useHotkeysStore.getState().hotkeysState; - // Use structural comparison that's order-independent - const currentStr = JSON.stringify( - current, - Object.keys(current).sort(), - ); - const newStr = JSON.stringify(state, Object.keys(state).sort()); - if (currentStr === newStr) { + // Use deep equality for reliable comparison + if (isEqual(current, state)) { return; }
331-363: Spreadingdepsinto the dependency array is fragile.Spreading a user-provided array into
useEffect's dependency array ([enabled, keys, preventDefault, ...deps]) bypasses ESLint'sexhaustive-depsrule and can lead to subtle bugs if callers pass unstable references. Consider either:
- Removing
depsand documenting thatcallbackshould be stable (or useuseCallbackat the call site).- Using
useReffor the callback (already done) and removing the external deps parameter entirely.Since
callbackRef.currentis updated on every render (line 341), the externaldepsarray is likely unnecessary.🔎 Proposed simplification
export function useAppHotkey( id: HotkeyId, callback: (event: KeyboardEvent, handler: unknown) => void, options?: { enabled?: boolean; preventDefault?: boolean }, - deps: unknown[] = [], ) { const keys = useHotkeyKeys(id); const enabled = Boolean(keys) && (options?.enabled ?? true); const preventDefault = options?.preventDefault ?? false; const callbackRef = useRef(callback); callbackRef.current = callback; useEffect(() => { if (!enabled || !keys) return; if ( typeof document === "undefined" || typeof document.addEventListener !== "function" ) { return; } const onKeyDown = (event: KeyboardEvent) => { if (!matchesHotkeyEvent(event, keys)) return; if (preventDefault) event.preventDefault(); callbackRef.current(event, undefined); }; document.addEventListener("keydown", onKeyDown); return () => { document.removeEventListener("keydown", onKeyDown); }; - }, [enabled, keys, preventDefault, ...deps]); + }, [enabled, keys, preventDefault]); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/shared/hotkeys.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
- apps/desktop/src/shared/hotkeys.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type safety and avoid
anytypes unless absolutely necessary in TypeScript files
Files:
apps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
apps/desktop/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Move Node.js functionality needed in Electron renderer to
src/main/lib/and communicate via IPC
Files:
apps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧠 Learnings (4)
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
Applied to files:
apps/desktop/src/renderer/stores/hotkeys/store.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/{shared/ipc-channels.ts,main/**/*ipcs.ts,renderer/**/*.tsx} : Define all Electron IPC channel types in `apps/desktop/src/shared/ipc-channels.ts` before implementing handlers
Applied to files:
apps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/**/*.{ts,tsx} : Move Node.js functionality needed in Electron renderer to `src/main/lib/` and communicate via IPC
Applied to files:
apps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
🧬 Code graph analysis (2)
apps/desktop/src/renderer/stores/hotkeys/store.ts (2)
apps/desktop/src/shared/hotkeys.ts (17)
HotkeysState(34-37)HotkeyPlatform(8-8)HotkeyId(30-30)HOTKEYS_STATE_VERSION(46-46)getCurrentPlatform(169-173)canonicalizeHotkeyForPlatform(181-189)hasPrimaryModifier(317-320)getDefaultHotkey(571-576)getEffectiveHotkey(578-585)formatHotkeyDisplay(191-208)formatHotkeyText(210-219)getEffectiveHotkeysMap(587-596)HotkeyCategory(10-15)HotkeyDefinition(17-28)HOTKEYS(365-544)isTerminalReservedHotkey(290-294)isOsReservedHotkey(303-310)apps/desktop/src/renderer/lib/trpc-storage.ts (2)
trpcHotkeysStorage(77-93)setSkipNextHotkeysPersist(11-13)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
apps/desktop/src/shared/hotkeys.ts (2)
isTerminalReservedEvent(296-301)matchesHotkeyEvent(221-254)apps/desktop/src/renderer/stores/hotkeys/store.ts (2)
getHotkeyKeys(196-200)isAppHotkeyEvent(251-260)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (5)
apps/desktop/src/renderer/stores/hotkeys/store.ts (2)
68-184: LGTM on the Zustand store setup.The store correctly uses Zustand with
devtoolsandpersistmiddleware, and leveragestrpcHotkeysStoragefor persistence via tRPC, which aligns with the coding guidelines. The validation logic insetHotkeyandsetHotkeysBatchproperly enforces canonicalization and the primary modifier requirement.
186-200: Good separation of hook vs. non-hook access patterns.Providing both
useHotkeyKeys(reactive hook) andgetHotkeyKeys(imperative access viagetState()) is a clean pattern that supports different use cases—React components and non-React code paths (like the terminal keyboard handler).apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (3)
11-13: Imports correctly updated for the new hotkeys system.The imports align with the new architecture:
getHotkeyKeysandisAppHotkeyEventfrom the renderer store for runtime hotkey resolution, andisTerminalReservedEventandmatchesHotkeyEventfrom shared utilities for event matching.
254-258: Clear shortcut logic correctly handles unassigned hotkey.When
getHotkeyKeys("CLEAR_TERMINAL")returnsnull(unassigned),isClearShortcutwill befalse, effectively disabling the clear functionality. This is the expected behavior for an unassigned hotkey.The early return on line 254 for
isTerminalReservedEvent(event)ensures terminal-native shortcuts (Ctrl+C, etc.) are not intercepted.
267-290: App hotkey forwarding logic is sound.The flow correctly:
- Ignores non-keydown events
- Requires a primary modifier (meta/ctrl)
- Re-dispatches matching app hotkeys to document for global handling
This ensures terminal doesn't swallow app-level shortcuts while still allowing terminal-native key sequences.
Merged displayLabel fallback from main with configurable shortcuts from configurable-keyboard-shortcuts branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx (2)
176-178: Remove hardcoded shortcut and apply dynamic pattern.The VS Code submenu still displays a hardcoded
⌘Oshortcut, which breaks the configurable shortcuts feature introduced in this PR. When users customize the OPEN_IN_APP shortcut, this submenu will continue showing the outdated binding. Additionally, the macOS-specific glyph won't display correctly on Windows/Linux.Apply the same conditional rendering pattern used in lines 150-152.
🔎 Proposed fix
{app.label} - {app.id === lastUsedApp && ( - <DropdownMenuShortcut>⌘O</DropdownMenuShortcut> + {app.id === lastUsedApp && showOpenInShortcut && ( + <DropdownMenuShortcut>{openInShortcut}</DropdownMenuShortcut> )}
193-205: Add shortcut indicators to JetBrains submenu for consistency.For a consistent user experience, the JetBrains submenu should display shortcut indicators matching the pattern used in the main menu (lines 150-152) and VS Code submenu (lines 176-178). Currently, users see shortcuts for VS Code variants but not for JetBrains apps.
🔎 Suggested enhancement
{app.label} + {app.id === lastUsedApp && showOpenInShortcut && ( + <DropdownMenuShortcut>{openInShortcut}</DropdownMenuShortcut> + )} </DropdownMenuItem>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsxapps/desktop/src/renderer/screens/main/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx
- apps/desktop/src/renderer/screens/main/index.tsx
🧰 Additional context used
📓 Path-based instructions (5)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type safety and avoid
anytypes unless absolutely necessary in TypeScript files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx
apps/**/src/**/**/[A-Z]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Structure component folders with one component per file using format
ComponentName/ComponentName.tsxwithindex.tsbarrel export
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx
apps/desktop/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Move Node.js functionality needed in Electron renderer to
src/main/lib/and communicate via IPC
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx
apps/desktop/src/{shared/ipc-channels.ts,main/**/*ipcs.ts,renderer/**/*.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Define all Electron IPC channel types in
apps/desktop/src/shared/ipc-channels.tsbefore implementing handlers
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx (1)
apps/desktop/src/renderer/stores/hotkeys/store.ts (1)
useHotkeyText(208-217)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/WorkspaceActionBar/components/WorkspaceActionBarRight/WorkspaceActionBarRight.tsx (4)
26-26: LGTM!The
useHotkeyTexthook integration follows best practices. The shortcut text and visibility flags are computed cleanly at the component level, enabling dynamic display based on user configuration.Also applies to: 69-72
117-117: LGTM!The conditional shortcut display in the tooltip provides clear feedback: shows the actual shortcut when assigned or an em dash when unassigned.
150-152: LGTM!The conditional rendering correctly shows the shortcut indicator only for the currently selected app and only when a shortcut is assigned. This provides clear visual feedback without cluttering the UI.
212-214: LGTM!The conditional rendering for the Copy Path shortcut follows the same clean pattern as the Open In App shortcut, ensuring consistency across the UI.
Kitenite
left a comment
Kitenite
left a comment
There was a problem hiding this comment.
Holy cow this is fantastic! Amazing work @andreasasprou
2 participants
Why
hotkeys-demo.mp4
What / How
This PR adds configurable keyboard shortcuts to
@superset/desktopand surfaces them consistently in the UI:apps/desktop/src/shared/hotkeys.ts.hotkeysStateto the persisted app state (lowdb-backed) and emits change events.uiState.hotkeys.get/set/subscribeover tRPC so the renderer can persist and live-sync without restart.hotkeystRPC router to export/import a JSON hotkeys file via native dialogs.useHotkeyDisplay,useHotkeyText,useAppHotkey, etc.) and wires common actions to configurable IDs (workspace switching, split pane, close pane, terminal actions, open-in-app, copy path, etc.).HotkeyTooltipContentfor consistent “Label + KbdGroup” rendering.bunfig.tomlto force Bun’sisolatedlinker to avoid a hoisting edge case that can break the@superset/desktoppostinstall native rebuild.Not in this PR
Files
apps/desktop/src/shared/hotkeys.tsapps/desktop/src/shared/hotkeys.test.tsapps/desktop/src/main/lib/hotkeys-events.tsapps/desktop/src/main/lib/menu.tsapps/desktop/src/main/lib/app-state/schemas.tsapps/desktop/src/lib/trpc/routers/ui-state/index.tsapps/desktop/src/lib/trpc/routers/hotkeys/index.tsapps/desktop/src/renderer/stores/hotkeys/store.tsapps/desktop/src/renderer/components/HotkeyTooltipContent/HotkeyTooltipContent.tsxbunfig.tomlTesting
bun run lintbun run typecheckcd apps/desktop && bun testSummary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.