Add keyboard shortcuts for jumping to sidebar threads#1456
Add keyboard shortcuts for jumping to sidebar threads#1456
Conversation
- Map the first nine visible threads to number keys - Show platform-specific shortcut hints while modifier is held - Add tests for key mapping and modifier detection
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
bot
added
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/src/components/Sidebar.logic.test.ts (1)
110-147: Good platform-aware modifier detection tests.Tests correctly cover:
- macOS with
metaKey- Windows with
ctrlKey- Rejection when
shiftKeyis also pressedConsider adding a test case for
altKey: trueto verify it's also rejected, matching the implementation requirement.📝 Optional: Add altKey rejection test
expect( isThreadJumpModifierPressed( { key: "Control", metaKey: false, ctrlKey: true, shiftKey: false, altKey: true, }, "Win32", ), ).toBe(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/Sidebar.logic.test.ts` around lines 110 - 147, Add a test asserting that isThreadJumpModifierPressed rejects when altKey is true: in the test "detects the active jump modifier by platform" add a case calling isThreadJumpModifierPressed with a Windows platform ("Win32") and an event object where ctrlKey: true, altKey: true (and shiftKey: false, metaKey: false) and expect the result to be false; this mirrors the existing shiftKey rejection case and verifies altKey is also treated as an invalid modifier.apps/web/src/components/Sidebar.logic.ts (1)
81-111: Defaultnavigator.platformmay throw in non-browser contexts.While Sidebar.tsx always passes
platformexplicitly, the default parameternavigator.platformwould throw aReferenceErrorif these helpers are ever called without an argument in SSR or test environments wherenavigatoris undefined.Consider using a safer default or documenting that the argument is required outside browser contexts.
🛡️ Suggested defensive default
export function isThreadJumpModifierPressed( event: ThreadJumpEvent, - platform = navigator.platform, + platform = typeof navigator !== "undefined" ? navigator.platform : "", ): boolean {Apply the same pattern to
resolveThreadJumpIndexandformatThreadJumpHintLabel.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/Sidebar.logic.ts` around lines 81 - 111, Defaulting parameters to navigator.platform can throw in non-browser environments; update isThreadJumpModifierPressed, resolveThreadJumpIndex, and formatThreadJumpHintLabel to use a safe platform default (e.g. const safePlatform = typeof navigator !== "undefined" ? navigator.platform : "" or undefined) instead of directly using navigator.platform in the parameter list, then use safePlatform in calls to isMacPlatform and other logic so these helpers won’t ReferenceError in SSR/tests.
🤖 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/web/src/components/Sidebar.logic.test.ts`:
- Around line 110-147: Add a test asserting that isThreadJumpModifierPressed
rejects when altKey is true: in the test "detects the active jump modifier by
platform" add a case calling isThreadJumpModifierPressed with a Windows platform
("Win32") and an event object where ctrlKey: true, altKey: true (and shiftKey:
false, metaKey: false) and expect the result to be false; this mirrors the
existing shiftKey rejection case and verifies altKey is also treated as an
invalid modifier.
In `@apps/web/src/components/Sidebar.logic.ts`:
- Around line 81-111: Defaulting parameters to navigator.platform can throw in
non-browser environments; update isThreadJumpModifierPressed,
resolveThreadJumpIndex, and formatThreadJumpHintLabel to use a safe platform
default (e.g. const safePlatform = typeof navigator !== "undefined" ?
navigator.platform : "" or undefined) instead of directly using
navigator.platform in the parameter list, then use safePlatform in calls to
isMacPlatform and other logic so these helpers won’t ReferenceError in
SSR/tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f3c49538-55ac-433c-86b5-46df99b9e76a
📒 Files selected for processing (3)
apps/web/src/components/Sidebar.logic.test.tsapps/web/src/components/Sidebar.logic.tsapps/web/src/components/Sidebar.tsx
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| }); | ||
| }, | ||
| [clearSelection, navigate, selectedThreadIds.size, setSelectionAnchor], | ||
| ); |
There was a problem hiding this comment.
Duplicated navigation logic in handleThreadClick and navigateToThread
Low Severity
The new navigateToThread callback duplicates the plain-click branch of the existing handleThreadClick callback (clear selection, set anchor, navigate). The plain-click path in handleThreadClick (lines 964–972) performs the exact same sequence — check selectedThreadIds.size, call clearSelection, setSelectionAnchor, and navigate — but was not updated to call navigateToThread. This duplication means a future change to the navigation logic risks being applied in one place but missed in the other.
Additional Locations (1)
- Bind Cmd/Ctrl+1-9 to thread jump commands - Resolve jump targets from visible sidebar threads - Show shortcut hints using current keybinding labels
1 participant
Summary
Cmd+1-Cmd+9on macOS,Ctrl+1-Ctrl+9elsewhere).Testing
Not runNote
Medium Risk
Adds new global
windowkeyboard listeners and expands the supported keybinding command set, which could introduce shortcut conflicts or unexpected navigation if matching logic is wrong.Overview
Adds default keybindings and contract support for
thread.jump.1throughthread.jump.9, enabling Cmd/Ctrl+number shortcuts.Implements sidebar thread-jump navigation: computes the first 9 visible thread targets, intercepts matching shortcut events to navigate (reusing the existing selection/anchor behavior), and shows inline shortcut hint badges while the modifier key is held. Includes unit tests for jump target selection and command index parsing.
Written by Cursor Bugbot for commit 6aadae6. This will update automatically on new commits. Configure here.
Note
Add keyboard shortcuts for jumping to sidebar threads
mod+1throughmod+9navigates to the corresponding visible thread in the sidebar, with per-thread hint badges shown while the modifier key is held.getVisibleThreadJumpTargetsin Sidebar.logic.ts to compute an ordered list of up to 9 visible thread IDs from the current sidebar state, skipping collapsed projects and those where the thread panel is hidden.parseThreadJumpIndexin keybindings.ts to parsethread.jump.[1-9]commands into zero-based indices.mod+1–mod+9are registered in keybindings.ts and the commands are added toSTATIC_KEYBINDING_COMMANDS.Macroscope summarized 6aadae6.
Summary by CodeRabbit