feat(desktop/hotkeys): unbound defaults + restore prev/next tab & workspace#3422
Merged
Conversation
…workspace Widen PlatformKey and HotkeyDefinition so hotkey entries can register with a null chord per platform. Downstream consumers were already null-safe from #3391 (useBinding, buildRegisteredAppChords, formatHotkeyDisplay, sanitizeOverride, HotkeyMenuShortcut), so the schema widening is the only structural change needed. Re-introduce PREV_TAB, NEXT_TAB, PREV_WORKSPACE, NEXT_WORKSPACE as registered-but-unbound entries so users who want tab/workspace neighbor navigation can rebind them in Settings → Keyboard. PR #3403 removed these to free Cmd+Alt+Arrow for directional pane focus; this restores the hotkey IDs (and their v1/v2 handlers) without claiming any default chord. Users with pre-#3403 overrides for these IDs will transparently get their bindings back since the override is preserved in localStorage. - Null-guard canonicalizeChord(defaultKey) in useRecordHotkeys so recording a new chord for an unbound hotkey no longer throws. - Replace the force-cast in resolveHotkeyFromEvent.test.ts sample picker with a type predicate so sampleDef.key narrows to string honestly instead of lying about the widened schema.
Contributor
📝 WalkthroughWalkthroughAdds four navigation hotkeys (prev/next workspace, prev/next tab), makes hotkey key values nullable in types/registry, strengthens test narrowing for nullable keys, and adds a null-check in hotkey recording logic to avoid canonicalization when default keys are missing. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Event as Hotkey Event
participant Handler as Hotkey Handler
participant State as State/Store
participant Nav as Navigator
User->>Event: Press PREV_TAB / NEXT_TAB or PREV_WORKSPACE / NEXT_WORKSPACE
Event->>Handler: Invoke registered handler
Handler->>State: Read current workspace/tab and indices
Handler->>State: Compute previous/next index (with wrap-around)
Handler->>Nav: Request navigation or set active tab
Nav-->>User: UI updates to selected workspace/tab
sequenceDiagram
participant User as User
participant WSHandler as Workspace Hotkey Handler
participant TRPC as TRPC Query
participant Nav as navigateToWorkspace
participant Router as Router
User->>WSHandler: Press PREV_WORKSPACE / NEXT_WORKSPACE
WSHandler->>TRPC: getPreviousWorkspace / getNextWorkspace
TRPC-->>WSHandler: Return adjacent workspace ID (or null)
alt adjacent workspace ID exists
WSHandler->>Nav: Call navigateToWorkspace(id)
Nav->>Router: Update route
Router-->>User: Render new workspace
else no adjacent workspace
WSHandler-->>User: No navigation (or handled by wrap logic)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
Greptile SummaryThis PR restores Key changes:
Two minor issues flagged: an asymmetric Confidence Score: 4/5Safe to merge; the core schema change and null-deref fix are surgical and correct, with only minor edge-case inconsistencies in the navigation handlers. The null-key schema widening is clean and propagated consistently. The useRecordHotkeys fix is correct. The new hotkey handlers for both v1 and v2 workspaces are logically sound. Two P2 issues exist: an asymmetric -1 fallback in PREV_WORKSPACE vs NEXT_WORKSPACE in the sidebar hook, and a silent no-op window in the v1 page while tRPC queries load — neither breaks the primary user path. useDashboardSidebarShortcuts.ts (asymmetric index -1 handling) and workspace/$workspaceId/page.tsx (tRPC query loading window + redundant enabled guard) Important Files Changed
Reviews (1): Last reviewed commit: "feat(desktop/hotkeys): allow unbound def..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+59
to
+66
| useHotkey("PREV_WORKSPACE", () => { | ||
| if (!currentWorkspaceId || flattenedWorkspaces.length === 0) return; | ||
| const index = flattenedWorkspaces.findIndex( | ||
| (w) => w.id === currentWorkspaceId, | ||
| ); | ||
| const prevIndex = index <= 0 ? flattenedWorkspaces.length - 1 : index - 1; | ||
| navigateToV2Workspace(flattenedWorkspaces[prevIndex].id, navigate); | ||
| }); |
There was a problem hiding this comment.
When findIndex returns -1 (the current workspace has creationStatus set and is therefore filtered out of flattenedWorkspaces), index <= 0 evaluates to true and prevIndex becomes flattenedWorkspaces.length - 1 (last workspace). The NEXT_WORKSPACE handler below explicitly guards index === -1 and sends the user to the first workspace instead. The asymmetry means the two hotkeys disagree on where to land for the same edge-case input.
Consider mirroring the explicit -1 guard here:
Suggested change
| useHotkey("PREV_WORKSPACE", () => { | |
| if (!currentWorkspaceId || flattenedWorkspaces.length === 0) return; | |
| const index = flattenedWorkspaces.findIndex( | |
| (w) => w.id === currentWorkspaceId, | |
| ); | |
| const prevIndex = index <= 0 ? flattenedWorkspaces.length - 1 : index - 1; | |
| navigateToV2Workspace(flattenedWorkspaces[prevIndex].id, navigate); | |
| }); | |
| useHotkey("PREV_WORKSPACE", () => { | |
| if (!currentWorkspaceId || flattenedWorkspaces.length === 0) return; | |
| const index = flattenedWorkspaces.findIndex( | |
| (w) => w.id === currentWorkspaceId, | |
| ); | |
| if (index === -1) { | |
| navigateToV2Workspace(flattenedWorkspaces[0].id, navigate); | |
| return; | |
| } | |
| const prevIndex = index <= 0 ? flattenedWorkspaces.length - 1 : index - 1; | |
| navigateToV2Workspace(flattenedWorkspaces[prevIndex].id, navigate); | |
| }); |
Alternatively, bail out when index === -1 (do nothing) if navigating away from a workspace-being-created feels undesirable. Either way, the behavior should be intentional and consistent between the two handlers.
Comment on lines
+412
to
+432
| const getPreviousWorkspace = | ||
| electronTrpc.workspaces.getPreviousWorkspace.useQuery( | ||
| { id: workspaceId }, | ||
| { enabled: !!workspaceId }, | ||
| ); | ||
| useHotkey("PREV_WORKSPACE", () => { | ||
| const prevWorkspaceId = getPreviousWorkspace.data; | ||
| if (prevWorkspaceId) { | ||
| navigateToWorkspace(prevWorkspaceId, navigate); | ||
| } | ||
| }); | ||
|
|
||
| const getNextWorkspace = electronTrpc.workspaces.getNextWorkspace.useQuery( | ||
| { id: workspaceId }, | ||
| { enabled: !!workspaceId }, | ||
| ); | ||
| useHotkey("NEXT_WORKSPACE", () => { | ||
| const nextWorkspaceId = getNextWorkspace.data; | ||
| if (nextWorkspaceId) { | ||
| navigateToWorkspace(nextWorkspaceId, navigate); | ||
| } |
There was a problem hiding this comment.
getPreviousWorkspace.data and getNextWorkspace.data are undefined until the IPC queries resolve. If the user presses PREV_WORKSPACE or NEXT_WORKSPACE within that window (e.g. immediately after navigating to a workspace), the handler returns without side-effects and gives no feedback. For most users on a warm cache this window is imperceptible, but it can surface as a confusing "hotkey didn't work" on first load.
A minimal improvement is to check getPreviousWorkspace.isLoading and either disable the hotkey registration or show a loading indicator, though this may be over-engineering for a desktop-only path. Worth documenting the known limitation at minimum.
Also: { enabled: !!workspaceId } is always true because workspaceId is a required route param that is never an empty string. The guard can be simplified to { enabled: true } or removed entirely.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx (1)
417-433: Fetch adjacent workspace on keypress to avoid first-press no-op/stale data.These handlers currently depend on
useQuery().data. Triggeringrefetch()in the hotkey path gives more reliable navigation state.♻️ Suggested refactor
useHotkey("PREV_WORKSPACE", () => { - const prevWorkspaceId = getPreviousWorkspace.data; - if (prevWorkspaceId) { - navigateToWorkspace(prevWorkspaceId, navigate); - } + void getPreviousWorkspace.refetch().then(({ data: prevWorkspaceId }) => { + if (prevWorkspaceId) { + void navigateToWorkspace(prevWorkspaceId, navigate); + } + }); }); useHotkey("NEXT_WORKSPACE", () => { - const nextWorkspaceId = getNextWorkspace.data; - if (nextWorkspaceId) { - navigateToWorkspace(nextWorkspaceId, navigate); - } + void getNextWorkspace.refetch().then(({ data: nextWorkspaceId }) => { + if (nextWorkspaceId) { + void navigateToWorkspace(nextWorkspaceId, navigate); + } + }); });🤖 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/workspace/`$workspaceId/page.tsx around lines 417 - 433, The hotkey handlers rely on stale useQuery().data; change the handlers for useHotkey("PREV_WORKSPACE", ...) and useHotkey("NEXT_WORKSPACE", ...) to call and await the corresponding query's refetch() (getPreviousWorkspace.refetch() and getNextWorkspace.refetch()) inside the handler, then read the refetch result to get the workspace id and call navigateToWorkspace(workspaceId, navigate); ensure you handle the promise result and falsy responses to avoid no-op presses.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceHotkeys/useWorkspaceHotkeys.ts (1)
90-122: Extract a shared tab-cycle helper to keep hotkey behavior in sync.
PREV_TAB/NEXT_TABduplicate the same wrap logic already used byPREV_TAB_ALT/NEXT_TAB_ALT. A single helper reduces drift risk.♻️ Suggested refactor
+ const cycleActiveTab = useCallback( + (direction: "prev" | "next") => { + const state = store.getState(); + if (!state.activeTabId || state.tabs.length === 0) return; + const index = state.tabs.findIndex((t) => t.id === state.activeTabId); + const targetIndex = + direction === "prev" + ? index <= 0 + ? state.tabs.length - 1 + : index - 1 + : index >= state.tabs.length - 1 || index === -1 + ? 0 + : index + 1; + state.setActiveTab(state.tabs[targetIndex].id); + }, + [store], + ); + - useHotkey("PREV_TAB", () => { - const state = store.getState(); - if (!state.activeTabId || state.tabs.length === 0) return; - const index = state.tabs.findIndex((t) => t.id === state.activeTabId); - const prevIndex = index <= 0 ? state.tabs.length - 1 : index - 1; - state.setActiveTab(state.tabs[prevIndex].id); - }); + useHotkey("PREV_TAB", () => cycleActiveTab("prev")); - useHotkey("NEXT_TAB", () => { - const state = store.getState(); - if (!state.activeTabId || state.tabs.length === 0) return; - const index = state.tabs.findIndex((t) => t.id === state.activeTabId); - const nextIndex = - index >= state.tabs.length - 1 || index === -1 ? 0 : index + 1; - state.setActiveTab(state.tabs[nextIndex].id); - }); + useHotkey("NEXT_TAB", () => cycleActiveTab("next")); - useHotkey("PREV_TAB_ALT", () => { - const state = store.getState(); - if (!state.activeTabId || state.tabs.length === 0) return; - const index = state.tabs.findIndex((t) => t.id === state.activeTabId); - const prevIndex = index <= 0 ? state.tabs.length - 1 : index - 1; - state.setActiveTab(state.tabs[prevIndex].id); - }); + useHotkey("PREV_TAB_ALT", () => cycleActiveTab("prev")); - useHotkey("NEXT_TAB_ALT", () => { - const state = store.getState(); - if (!state.activeTabId || state.tabs.length === 0) return; - const index = state.tabs.findIndex((t) => t.id === state.activeTabId); - const nextIndex = - index >= state.tabs.length - 1 || index === -1 ? 0 : index + 1; - state.setActiveTab(state.tabs[nextIndex].id); - }); + useHotkey("NEXT_TAB_ALT", () => cycleActiveTab("next"));🤖 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/v2-workspace/`$workspaceId/hooks/useWorkspaceHotkeys/useWorkspaceHotkeys.ts around lines 90 - 122, Hotkey handlers PREV_TAB/NEXT_TAB duplicate the wrap-around logic in PREV_TAB_ALT/NEXT_TAB_ALT; extract a shared helper (e.g., cycleTab(direction) or getWrappedIndex(state, direction)) that reads store.getState(), returns the wrapped target tab id (handling empty tabs, missing activeTabId, and index === -1 the same way as current code), and calls state.setActiveTab(id); then replace each useHotkey callback (PREV_TAB, NEXT_TAB, PREV_TAB_ALT, NEXT_TAB_ALT) to call that helper with a "prev" or "next" direction so all four hotkeys share the same logic and remain in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useWorkspaceHotkeys/useWorkspaceHotkeys.ts:
- Around line 90-122: Hotkey handlers PREV_TAB/NEXT_TAB duplicate the
wrap-around logic in PREV_TAB_ALT/NEXT_TAB_ALT; extract a shared helper (e.g.,
cycleTab(direction) or getWrappedIndex(state, direction)) that reads
store.getState(), returns the wrapped target tab id (handling empty tabs,
missing activeTabId, and index === -1 the same way as current code), and calls
state.setActiveTab(id); then replace each useHotkey callback (PREV_TAB,
NEXT_TAB, PREV_TAB_ALT, NEXT_TAB_ALT) to call that helper with a "prev" or
"next" direction so all four hotkeys share the same logic and remain in sync.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`$workspaceId/page.tsx:
- Around line 417-433: The hotkey handlers rely on stale useQuery().data; change
the handlers for useHotkey("PREV_WORKSPACE", ...) and
useHotkey("NEXT_WORKSPACE", ...) to call and await the corresponding query's
refetch() (getPreviousWorkspace.refetch() and getNextWorkspace.refetch()) inside
the handler, then read the refetch result to get the workspace id and call
navigateToWorkspace(workspaceId, navigate); ensure you handle the promise result
and falsy responses to avoid no-op presses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc027b9d-6d0e-4ec1-a0fc-d3b3d7b0240f
📒 Files selected for processing (7)
apps/desktop/src/renderer/hotkeys/hooks/useRecordHotkeys/useRecordHotkeys.tsapps/desktop/src/renderer/hotkeys/registry.tsapps/desktop/src/renderer/hotkeys/types.tsapps/desktop/src/renderer/hotkeys/utils/resolveHotkeyFromEvent.test.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceHotkeys/useWorkspaceHotkeys.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 7 files
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/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.ts:64">
P2: Handle `findIndex` returning `-1` in PREV_WORKSPACE before navigating; otherwise it can unexpectedly jump to the last workspace.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.ts (1)
59-77: Consider extracting shared index lookup/guard logic to reduce duplication.Both handlers repeat the same preconditions and
findIndexpath. A small helper keeps behavior in sync and simplifies future edits.♻️ Optional refactor
+ const getCurrentWorkspaceIndex = useCallback(() => { + if (!currentWorkspaceId || flattenedWorkspaces.length === 0) return -1; + return flattenedWorkspaces.findIndex((w) => w.id === currentWorkspaceId); + }, [currentWorkspaceId, flattenedWorkspaces]); + useHotkey("PREV_WORKSPACE", () => { - if (!currentWorkspaceId || flattenedWorkspaces.length === 0) return; - const index = flattenedWorkspaces.findIndex( - (w) => w.id === currentWorkspaceId, - ); + const index = getCurrentWorkspaceIndex(); if (index === -1) return; const prevIndex = index <= 0 ? flattenedWorkspaces.length - 1 : index - 1; navigateToV2Workspace(flattenedWorkspaces[prevIndex].id, navigate); }); useHotkey("NEXT_WORKSPACE", () => { - if (!currentWorkspaceId || flattenedWorkspaces.length === 0) return; - const index = flattenedWorkspaces.findIndex( - (w) => w.id === currentWorkspaceId, - ); + const index = getCurrentWorkspaceIndex(); if (index === -1) return; const nextIndex = index >= flattenedWorkspaces.length - 1 ? 0 : index + 1; navigateToV2Workspace(flattenedWorkspaces[nextIndex].id, navigate); });🤖 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 59 - 77, Extract the repeated guard and index-lookup into a small helper used by both hotkey handlers: create a function (e.g., getCurrentWorkspaceIndex or withCurrentWorkspaceIndex) that checks currentWorkspaceId and flattenedWorkspaces length, performs flattenedWorkspaces.findIndex(w => w.id === currentWorkspaceId) and returns -1 or the index (or invokes a callback only when index !== -1); then replace the duplicated blocks inside the useHotkey handlers for PREV_WORKSPACE and NEXT_WORKSPACE to call that helper and compute prev/next indices before calling navigateToV2Workspace(..., navigate). This keeps logic shared between useHotkey("PREV_WORKSPACE", ...) and useHotkey("NEXT_WORKSPACE", ...) and preserves use of currentWorkspaceId, flattenedWorkspaces, navigateToV2Workspace, and navigate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.ts`:
- Around line 59-77: Extract the repeated guard and index-lookup into a small
helper used by both hotkey handlers: create a function (e.g.,
getCurrentWorkspaceIndex or withCurrentWorkspaceIndex) that checks
currentWorkspaceId and flattenedWorkspaces length, performs
flattenedWorkspaces.findIndex(w => w.id === currentWorkspaceId) and returns -1
or the index (or invokes a callback only when index !== -1); then replace the
duplicated blocks inside the useHotkey handlers for PREV_WORKSPACE and
NEXT_WORKSPACE to call that helper and compute prev/next indices before calling
navigateToV2Workspace(..., navigate). This keeps logic shared between
useHotkey("PREV_WORKSPACE", ...) and useHotkey("NEXT_WORKSPACE", ...) and
preserves use of currentWorkspaceId, flattenedWorkspaces, navigateToV2Workspace,
and navigate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c7b8b4e0-eb5c-4908-af99-1202bb959ede
📒 Files selected for processing (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useDashboardSidebarShortcuts/useDashboardSidebarShortcuts.ts
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
… null keys Pre-requisite for upstream superset-sh#3422 (unbound hotkey defaults). Allows hotkey entries to register with a null chord per platform. FORK NOTE: fork-specific hotkeys (BROWSER_RELOAD, BROWSER_HARD_RELOAD, SEARCH_IN_FILES) retain their non-null keys.
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
…kspace (superset-sh#3422) * feat(desktop/hotkeys): allow unbound defaults; restore PREV/NEXT tab+workspace Widen PlatformKey and HotkeyDefinition so hotkey entries can register with a null chord per platform. Downstream consumers were already null-safe from superset-sh#3391 (useBinding, buildRegisteredAppChords, formatHotkeyDisplay, sanitizeOverride, HotkeyMenuShortcut), so the schema widening is the only structural change needed. Re-introduce PREV_TAB, NEXT_TAB, PREV_WORKSPACE, NEXT_WORKSPACE as registered-but-unbound entries so users who want tab/workspace neighbor navigation can rebind them in Settings → Keyboard. PR superset-sh#3403 removed these to free Cmd+Alt+Arrow for directional pane focus; this restores the hotkey IDs (and their v1/v2 handlers) without claiming any default chord. Users with pre-superset-sh#3403 overrides for these IDs will transparently get their bindings back since the override is preserved in localStorage. - Null-guard canonicalizeChord(defaultKey) in useRecordHotkeys so recording a new chord for an unbound hotkey no longer throws. - Replace the force-cast in resolveHotkeyFromEvent.test.ts sample picker with a type predicate so sampleDef.key narrows to string honestly instead of lying about the widened schema. * fix(desktop/hotkeys): restore prevIndex in v2 PREV_WORKSPACE handler
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
…RKSPACE handlers Post-cherry-pick cleanup after superset-sh#3422: - registry.ts: remove old key-bound PREV_TAB/NEXT_TAB entries (meta+alt+left/right) now superseded by null-bound entries from upstream superset-sh#3422 - page.tsx: remove duplicate getPreviousWorkspace/getNextWorkspace tRPC queries and PREV_WORKSPACE/NEXT_WORKSPACE handlers added by cherry-pick (originals with { enabled: isActive } guards already present above) FORK NOTE: PREV/NEXT_WORKSPACE still use tRPC-based implementation in v1 workspace page.
5 tasks
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
…ff viewer Follow-up fixes for superset-sh#3403 and superset-sh#3420 cherry-picks: hotkeys/types.ts: - Widen PlatformKey and HotkeyDefinition.key to allow null per platform (mirrors upstream superset-sh#3422; needed here because v1 workspace uses fork's tRPC-based PREV/NEXT_WORKSPACE handlers that reference null-bound keys) hotkeys/registry.ts: - Re-add PREV_TAB, NEXT_TAB, PREV_WORKSPACE, NEXT_WORKSPACE as null-bound (matches upstream superset-sh#3422 final state; deleted by superset-sh#3403's auto-merge but fork still needs the IDs for v1 tRPC-based workspace nav handlers) hotkeys/useRecordHotkeys.ts: - Null-guard canonicalizeChord(defaultKey) so recording a new chord for an unbound hotkey doesn't throw routes/workspace/$workspaceId/page.tsx: - Restore navigateToWorkspace import (removed by cherry-pick auto-merge); fork's tRPC-based PREV/NEXT_WORKSPACE handlers still need it routes/v2-workspace/$workspaceId/page.tsx: - Remove tabTitle argument from openPane call (deprecated by superset-sh#3420; pane-level titleOverride handles tab titles now via resolveTabTitle) FORK NOTE: the registry entries and types.ts widening overlap with PR#2 (superset-sh#3422 cherry-pick). When both PRs merge, git detects identical changes and auto-resolves.
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
1. Parse file tab titles with Windows path separators (Codex P2)
- getFileName in usePaneRegistry.tsx split on / only, regressing Windows paths
like C:\repo\foo.ts which would render as the full path.
- Use split(/[/\\]/) to handle both separators cross-platform.
2. Restore PREV_TAB/NEXT_TAB handlers in v2 useWorkspaceHotkeys (Codex P2)
- superset-sh#3403 (cherry-picked here) removed these handlers, but superset-sh#3422 (PR#2)
restored them as null-bound in the registry. Without callbacks, users
rebinding these hotkeys in Settings would press them to no effect.
- Add handlers matching upstream c925f4d's restoration so override-based
tab navigation works in v2 workspaces.
Merged
13 tasks
MocA-Love
added a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
[PR2/5] feat(upstream): ホットキー型拡張 + Prev/Next Tab・Workspace の unbound 化 (superset-sh#3422)
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
…ff viewer Follow-up fixes for superset-sh#3403 and superset-sh#3420 cherry-picks: hotkeys/types.ts: - Widen PlatformKey and HotkeyDefinition.key to allow null per platform (mirrors upstream superset-sh#3422; needed here because v1 workspace uses fork's tRPC-based PREV/NEXT_WORKSPACE handlers that reference null-bound keys) hotkeys/registry.ts: - Re-add PREV_TAB, NEXT_TAB, PREV_WORKSPACE, NEXT_WORKSPACE as null-bound (matches upstream superset-sh#3422 final state; deleted by superset-sh#3403's auto-merge but fork still needs the IDs for v1 tRPC-based workspace nav handlers) hotkeys/useRecordHotkeys.ts: - Null-guard canonicalizeChord(defaultKey) so recording a new chord for an unbound hotkey doesn't throw routes/workspace/$workspaceId/page.tsx: - Restore navigateToWorkspace import (removed by cherry-pick auto-merge); fork's tRPC-based PREV/NEXT_WORKSPACE handlers still need it routes/v2-workspace/$workspaceId/page.tsx: - Remove tabTitle argument from openPane call (deprecated by superset-sh#3420; pane-level titleOverride handles tab titles now via resolveTabTitle) FORK NOTE: the registry entries and types.ts widening overlap with PR#2 (superset-sh#3422 cherry-pick). When both PRs merge, git detects identical changes and auto-resolves.
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
1. Parse file tab titles with Windows path separators (Codex P2)
- getFileName in usePaneRegistry.tsx split on / only, regressing Windows paths
like C:\repo\foo.ts which would render as the full path.
- Use split(/[/\\]/) to handle both separators cross-platform.
2. Restore PREV_TAB/NEXT_TAB handlers in v2 useWorkspaceHotkeys (Codex P2)
- superset-sh#3403 (cherry-picked here) removed these handlers, but superset-sh#3422 (PR#2)
restored them as null-bound in the registry. Without callbacks, users
rebinding these hotkeys in Settings would press them to no effect.
- Add handlers matching upstream c925f4d's restoration so override-based
tab navigation works in v2 workspaces.
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
All 9 upstream commits have been individually cherry-picked via PR#159~#163: | Upstream | Our PR | Description | |---|---|---| | d656b7e (superset-sh#3415) | #159 (PR#1) | terminal clipboard handling | | 31fcf19 (superset-sh#3416) | #162 (PR#4) | v1 split pane startup sizing fix | | 039edf2 (superset-sh#3403) | #161 (PR#3) | Cmd+Alt+Arrow spatial pane focus | | b18a00c (superset-sh#3421) | #159 (PR#1) | v2 right sidebar toggle reactive | | 3dd1de2 (superset-sh#3420) | #161 (PR#3) | v2 diff viewer + tab title resolution | | b42a114 (superset-sh#3418) | #159 (PR#1) | CodeMirror hotkey enablement | | c925f4d (superset-sh#3422) | #160 (PR#2) | unbound defaults + restore prev/next tab/workspace | | bb12c09 (superset-sh#3419) | #163 (PR#5) | version bump 1.5.3 | | 47efa73 (superset-sh#3432) | #159 (PR#1) | pending/update-required error selectable | Fork-specific features preserved: - auto-updater (IS_FORK, GitHub Releases API) - QuitMode/cleanupMainWindowResources lifecycle - GitHubSyncService, SpreadsheetViewer - BROWSER_RELOAD / BROWSER_HARD_RELOAD / SEARCH_IN_FILES hotkeys - HotkeyCategory "Browser" - v1 deep-link navigation (useSearch/WorkspaceSearchParams) - v1 tRPC-based PREV/NEXT_WORKSPACE handlers - v1 CLOSE_TERMINAL/CLOSE_TAB hotkey handlers - v2 extra state (rightSidebarOpenViewWidth, showPresetsBar)
6 tasks
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
PlatformKeyandHotkeyDefinition.keytostring | nullso registry entries can declare an unbound default per platform. All downstream consumers were already null-safe from fix(desktop): keyboard settings — Ctrl bindings, event.code unification, terminal override respect #3391, so this is a surgical schema change.PREV_TAB,NEXT_TAB,PREV_WORKSPACE,NEXT_WORKSPACEas registered-but-unbound entries with their v1/v2 handlers restored. Users can rebind them in Settings → Keyboard.useRecordHotkeys(canonicalizeChord(defaultKey)previously threw when a hotkey had no default) and replaces a type-assertion lie inresolveHotkeyFromEvent.test.tswith a proper type predicate so the sample picker narrows honestly against the widened schema.Follow-up to #3403, which retired these four hotkey IDs to free
Cmd+Alt+Arrowfor directional pane focus. Some users still want linear tab/workspace nav — they just don't need a default chord at install time. Users whose pre-#3403 overrides for these IDs survived in localStorage will transparently get their bindings back when this lands.Test plan
bun run typecheckcleanbun run lintcleanCmd+Ctrl+Left) for one of the unbound entries; confirm it persists, renders correctly, and the action actually fires in both v1 and v2 workspacesCmd+Alt+Arrowdirectional pane nav (from feat(desktop): Cmd+Alt+Arrow moves focus between v2 panes #3403) still worksSummary by cubic
Enable unassigned default hotkeys per platform and restore Previous/Next Tab and Workspace as registered-but-unbound entries. Users can bind them in Settings, and any existing overrides are restored automatically.
New Features
PlatformKeyandHotkeyDefinition.keytostring | nullto support unbound defaults.PREV_TAB,NEXT_TAB,PREV_WORKSPACE,NEXT_WORKSPACEwith null defaults; handlers work in both v1 and v2 workspaces.Bug Fixes
canonicalizeChord(defaultKey)inuseRecordHotkeysand fix v2PREV_WORKSPACEto use the correctprevIndex, preventing crashes and ensuring wrap-around works.resolveHotkeyFromEvent.test.tswith a type predicate to align with the widened schema.Written for commit 80eabc9. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes