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
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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.
| } | ||
|
|
||
| return mapping; | ||
| }, [renderedProjects]); |
There was a problem hiding this comment.
Jump keys assigned to threads in collapsed projects
Medium Severity
The threadJumpKeyById memo iterates over renderedProject.renderedThreads for every project, including those where shouldShowThreadPanel is false (i.e., collapsed projects with no active thread). These threads are hidden inside a closed Collapsible but still consume jump key slots 1–9. This causes visible threads in later projects to receive higher-numbered (or no) shortcuts, and pressing a shortcut for a hidden thread navigates to it without the user seeing it in the sidebar. The loop needs to skip projects where shouldShowThreadPanel is false.
| }); | ||
| }, | ||
| [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)
1 participant
Summary
Cmd+1-Cmd+9on macOS,Ctrl+1-Ctrl+9elsewhere).Testing
Not runNote
Medium Risk
Adds global
keydown/keyuplisteners to drive navigation and UI hint state, which could conflict with existing shortcuts or focus/selection behavior. Scope is limited to sidebar UX with unit tests covering the new helper logic.Overview
Adds platform-aware keyboard shortcuts to jump to the first nine visible sidebar threads (
Cmd+1..9on macOS,Ctrl+1..9elsewhere) and navigates using the same path as clicking a thread (via a sharednavigateToThreadhelper).Shows contextual shortcut hint pills next to eligible thread rows only while the modifier key is held, and refactors per-project/thread rendering into a memoized
renderedProjectsstructure to compute the visible-thread ordering used for jump-key assignment.Extracts jump-key parsing/labeling into new
Sidebar.logichelpers (getThreadJumpKey,isThreadJumpModifierPressed,resolveThreadJumpIndex,formatThreadJumpHintLabel) and adds unit tests for them.Written by Cursor Bugbot for commit aa91042. This will update automatically on new commits. Configure here.
Note
Add keyboard shortcuts for jumping to sidebar threads
Cmd(macOS) orCtrl(other platforms) in the sidebar reveals numeric hint badges (1–9) on visible threads across all projects.Cmd/Ctrl+1throughCmd/Ctrl+9navigates directly to the corresponding thread without mouse interaction.getThreadJumpKey), modifier detection (isThreadJumpModifierPressed), index resolution (resolveThreadJumpIndex), and label formatting (formatThreadJumpHintLabel).keydown/keyup/blurhandlers onwindowto show/hide hints and trigger navigation; thread navigation is refactored into a sharednavigateToThreadcallback.Macroscope summarized aa91042.
Summary by CodeRabbit