feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav#3472
feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav#3472saddlepaddle merged 1 commit intomainfrom
Conversation
Flip Cmd+Alt+Arrow (mac) / Ctrl+Shift+Alt+Arrow (win/linux) back to the pre-#3403 behavior: - Cmd+Alt+Left / Right → Previous / Next Tab - Cmd+Alt+Up / Down → Previous / Next Workspace FOCUS_PANE_{LEFT,RIGHT,UP,DOWN} remain registered but are now unbound by default — users who want directional pane focus can rebind them in Settings → Keyboard. PREV_TAB_ALT / NEXT_TAB_ALT (Ctrl+Tab / Ctrl+Shift+Tab) are unchanged. Also move SCROLL_TO_BOTTOM on Windows/Linux from ctrl+shift+alt+down to ctrl+end to avoid the silent collision with NEXT_WORKSPACE that this restoration would otherwise introduce. buildRegisteredAppChords uses a Map keyed by canonical chord, so duplicate chords silently clobber each other in the reverse lookup. mac SCROLL_TO_BOTTOM (meta+shift+down) is unchanged. Registered users who previously overrode any of these IDs keep their overrides — defaults are just fallbacks.
📝 WalkthroughWalkthroughUpdated hotkey bindings in the desktop application's registry for multiple navigation commands. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/hotkeys/registry.ts (1)
104-119: Add a regression test for per-platform default-chord uniqueness.This fix is entirely data-driven. A small test that canonicalizes each platform's non-null defaults and fails on duplicates would catch future silent overwrites in
buildRegisteredAppChordsbefore they ship.Also applies to: 329-333, 353-392
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/registry.ts` around lines 104 - 119, Add a regression test that ensures per-platform default chord keys are unique by canonicalizing non-null defaults for each platform (mac, windows, linux) across the hotkey registry entries (e.g., PREV_WORKSPACE, NEXT_WORKSPACE and other entries covered at lines ~329-333 and ~353-392) or by invoking buildRegisteredAppChords and inspecting its output; the test should collect each platform's key strings into a Set and fail/assert when a duplicate canonical key is detected so future data edits cannot silently overwrite per-platform chords.
🤖 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/hotkeys/registry.ts`:
- Around line 104-119: Add a regression test that ensures per-platform default
chord keys are unique by canonicalizing non-null defaults for each platform
(mac, windows, linux) across the hotkey registry entries (e.g., PREV_WORKSPACE,
NEXT_WORKSPACE and other entries covered at lines ~329-333 and ~353-392) or by
invoking buildRegisteredAppChords and inspecting its output; the test should
collect each platform's key strings into a Set and fail/assert when a duplicate
canonical key is detected so future data edits cannot silently overwrite
per-platform chords.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ea274ab3-190b-4224-b591-35f8ab396dc8
📒 Files selected for processing (1)
apps/desktop/src/renderer/hotkeys/registry.ts
Greptile SummaryThis PR restores
Confidence Score: 4/5Safe to merge; the one open question is whether ctrl+end is an acceptable default for SCROLL_TO_BOTTOM on Windows/Linux The tab/workspace restoration and the Map-collision fix are both correct and well-reasoned. The only concrete concern is the ctrl+end default for SCROLL_TO_BOTTOM — it resolves the previous collision but introduces a new potential conflict with a common ANSI terminal sequence. This is a trade-off worth discussing before merge, but it does not break the primary navigation paths restored by this PR and the user can rebind. apps/desktop/src/renderer/hotkeys/registry.ts — specifically the SCROLL_TO_BOTTOM Win/Linux default Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[KeyboardEvent fired] --> B[eventToChord]
B --> C{chord in registeredAppChords?}
C -- yes --> D[resolveHotkeyFromEvent returns HotkeyId]
C -- no --> E[null — passed through]
D --> F{HotkeyId}
F --> G[PREV_TAB / NEXT_TAB]
F --> H[PREV_WORKSPACE / NEXT_WORKSPACE]
F --> I[SCROLL_TO_BOTTOM ctrl+end Win/Linux]
F --> J[FOCUS_PANE_* null unbound]
G --> K[useWorkspaceHotkeys cycle tabs]
H --> L[useDashboardSidebarShortcuts cycle workspaces]
I --> M[useTerminalHotkeys scrollToBottom xterm]
J --> N[visible in Settings as Unassigned]
|
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…uperset-sh#3472) Flip Cmd+Alt+Arrow (mac) / Ctrl+Shift+Alt+Arrow (win/linux) back to the pre-superset-sh#3403 behavior: - Cmd+Alt+Left / Right → Previous / Next Tab - Cmd+Alt+Up / Down → Previous / Next Workspace FOCUS_PANE_{LEFT,RIGHT,UP,DOWN} remain registered but are now unbound by default — users who want directional pane focus can rebind them in Settings → Keyboard. PREV_TAB_ALT / NEXT_TAB_ALT (Ctrl+Tab / Ctrl+Shift+Tab) are unchanged. Also move SCROLL_TO_BOTTOM on Windows/Linux from ctrl+shift+alt+down to ctrl+end to avoid the silent collision with NEXT_WORKSPACE that this restoration would otherwise introduce. buildRegisteredAppChords uses a Map keyed by canonical chord, so duplicate chords silently clobber each other in the reverse lookup. mac SCROLL_TO_BOTTOM (meta+shift+down) is unchanged. Registered users who previously overrode any of these IDs keep their overrides — defaults are just fallbacks.
chore(upstream): PR5 hotkeys — v1 directional pane focus + Cmd+Alt+Arrow restoration (superset-sh#3460 superset-sh#3472)
-s ours merge to record that upstream commits a3e34bf through de70163 (13 commits) are semantically already present on origin/main via the PR1-6 cherry-pick series (PRs #176, #177, #178, #179, #180, #182), plus fork-adaptation fixes layered on top. This merge target is de70163 specifically (not upstream/main) so newer upstream commits (9fff075 and later) remain visible in future behind counts. Upstream commits covered by this audit merge: - a3e34bf fix(desktop): restore cmd+click requirement for v1 terminal file links (superset-sh#3457) [PR1/#176] - 57557f8 fix(desktop): gate v2 workspace children on collection readiness (superset-sh#3464) [PR1/#176] - 4ee2e61 fix(desktop): use native clipboard for copy path in v2 sidebar (superset-sh#3462) [PR1/#176] - 87d6e93 feat(desktop): close settings with Escape key (superset-sh#3466) [PR1/#176] - 9c7f5f4 chore(desktop): auto-restart host-service on bundle change in dev (superset-sh#3461) [PR1/#176] - 93140d9 fix(mcp): accept MCP resource URL as valid OAuth audience (superset-sh#3459) [PR2/#177] - be9e000 fix(desktop): drive tray menu off events, fetch real org name (superset-sh#3458) [PR2/#177] - c5f791e feat(v2): unify workspace delete through host-service (superset-sh#3443) [PR3/#178] - 2c24d93 feat(desktop): paginated branch picker with checkout + open actions (superset-sh#3397) [PR4/#179] - 2bf1049 feat(desktop/hotkeys): v1 directional pane focus + best-effort v1 override migrator (superset-sh#3460) [PR5/#180] - 1294a7d feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav (superset-sh#3472) [PR5/#180] - de70163 feat(desktop): v2 review tab first pass — PR info, checks, comments (superset-sh#3463) [PR6/#182] Intentionally skipped (version bump, fork has independent versioning): - 1e23353 chore(desktop): bump version to 1.5.5 (superset-sh#3473) Fork-adaptation fixes layered on top of the cherry-picks: - PR1: host-service-coordinator alias import fix, settings Escape selector narrowing (role-based + popper wrapper), Escape close uses replace navigation - PR2: dual quit mode preservation (requestQuit "release"/"stop"), trayUpdateToken guard for stale async fetchHostInfo results - PR4: ChangesHeader.normalizeBranchName regex rewrite (lint false positive), worktree add uses fullRef for remote-tracking refs, syncTimedOut reset on pendingId change, GIT_REFS.md barrel example fix - PR5: migrate.ts re-sanitize of existing localStorage overrides (v2 marker bump intent), FOCUS_PANE_* enabled:isActive for KeepAliveWorkspaces, CATEGORY_ORDER merges Navigation (upstream) and Browser (fork) - PR6: normalizeThreadsToComments flattens all thread.comments (not just first), CommentPane overrides <a> (openUrl) and <img> (SafeImage), zero-badge suppression, pr-null comments gate Fork features verified intact (Explore agent audit of combined 36d4de4..35d95f3 range): - BROWSER_RELOAD / BROWSER_HARD_RELOAD hotkeys - dual quit mode menu in tray - v1 terminal cold-restore + retry reconnect (out of range but unaffected) - KeepAliveWorkspaces (FOCUS_PANE_* gated on isActive) - useCommandPalette + addMemoTab in v2 workspace - host-service-coordinator rename alias pattern
1 participant
Summary
Flip the Cmd+Alt+Arrow (mac) / Ctrl+Shift+Alt+Arrow (win/linux) chords back to tab/workspace navigation, which is how they worked before #3403 retired them in favor of directional pane focus.
The SCROLL_TO_BOTTOM collision fix
buildRegisteredAppChordsuses aMapkeyed by canonical chord. When two hotkeys hash to the same chord, the later entry silently overwrites the earlier one in the reverse index — no error, the loser just stops responding.Before this PR, on Windows/Linux
SCROLL_TO_BOTTOMandFOCUS_PANE_DOWNboth claimedctrl+shift+alt+down, silently breakingFOCUS_PANE_DOWN(pre-existing latent bug — see the abandoned commit9fae2612dwhich proposed the same fix). RestoringNEXT_WORKSPACEtoctrl+shift+alt+downwould just move the bug from "breaks pane nav" to "breaks workspace nav".Moving
SCROLL_TO_BOTTOMon Windows/Linux fromctrl+shift+alt+downtoctrl+endresolves both. MacSCROLL_TO_BOTTOM(meta+shift+down) is untouched.User override safety
Any user who previously overrode
PREV_TAB,NEXT_TAB,PREV_WORKSPACE,NEXT_WORKSPACE, or anyFOCUS_PANE_*keeps their override — defaults are just fallbacks. Users on stock defaults get the new (restored) values.Test plan
bun run typecheck --filter @superset/desktopcleanSummary by CodeRabbit
New Features
Improvements
Summary by cubic
Restore Cmd+Alt+Arrow on mac and Ctrl+Shift+Alt+Arrow on Windows/Linux for tab and workspace navigation. Unbind pane-focus arrows by default and move SCROLL_TO_BOTTOM to Ctrl+End on Windows/Linux to avoid a collision.
New Features
Bug Fixes
Written for commit f643a96. Summary will update on new commits.