feat(sidebar): add chat number shortcuts#1730
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR implements numbered keyboard shortcuts (1–9, 0) for sidebar chat session selection. The feature adds macOS (⌘) and Windows/Linux (Alt) modifier support with visual badge overlays, keyboard event handling, and comprehensive test coverage across 22 languages. ChangesSidebar Chat Number Shortcuts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/renderer/components/WindowSideBarSessionItem.test.ts (1)
144-169: 💤 Low valueConsider verifying the data attribute is absent when badge is hidden.
The test comprehensively verifies that the badge and delete button render exclusively. However, when the badge is not visible (lines 160-169), you could add an assertion to confirm that the
data-shortcut-badge-visibleattribute is not present on the.right-buttonelement, mirroring the positive assertion at line 156-158.✨ Optional assertion to add
expect(normalWrapper.find('[data-testid="sidebar-session-shortcut-badge"]').exists()).toBe( false ) expect(normalWrapper.find('[aria-label="thread.actions.delete"]').exists()).toBe(true) + expect(normalWrapper.find('.right-button').attributes('data-shortcut-badge-visible')).toBeUndefined() }, 10000)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/renderer/components/WindowSideBarSessionItem.test.ts` around lines 144 - 169, In the test "renders shortcut badges independently from the delete action", add a negative assertion for the data attribute when the badge is hidden. After creating normalWrapper via mountComponent with shortcutBadgeVisible: false, verify that normalWrapper.find('.right-button') does not have the data-shortcut-badge-visible attribute (mirroring the positive check done on badgeWrapper for 'true'). Keep existing assertions for the absence of the badge and presence of the delete button intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/renderer/src/components/WindowSideBar.vue`:
- Around line 942-952: The isSidebarShortcutDigitEvent predicate is treating
auto-repeated keydown events as new shortcut invocations; add a guard that
ignores auto-repeat events (KeyboardEvent.repeat) so the shortcut only fires on
the initial keydown. Update the isSidebarShortcutDigitEvent function to return
false when event.repeat is true before checking modifiers and digit, keeping the
existing platform-specific modifier checks (shortcutPlatform.value === 'mac'
branch and the altKey branch) intact.
In `@test/renderer/components/WindowSideBar.test.ts`:
- Around line 553-926: Tests that dispatch real window keyboard events leave
WindowSideBar listeners registered, causing order-dependent failures; ensure
mounted wrappers are unmounted between tests by tearing down components returned
from setup. Update tests that use dispatchWindowKeydown or the WindowSideBar
behavior (references: setup(), WindowSideBar, dispatchWindowKeydown, wrapper) to
call wrapper.unmount() after use or add a global afterEach that tracks and
unmounts the wrapper returned by setup (or modify setup to register a
cleanup/unmount for the test harness). This guarantees window shortcut listeners
are removed before the next spec runs.
---
Nitpick comments:
In `@test/renderer/components/WindowSideBarSessionItem.test.ts`:
- Around line 144-169: In the test "renders shortcut badges independently from
the delete action", add a negative assertion for the data attribute when the
badge is hidden. After creating normalWrapper via mountComponent with
shortcutBadgeVisible: false, verify that normalWrapper.find('.right-button')
does not have the data-shortcut-badge-visible attribute (mirroring the positive
check done on badgeWrapper for 'true'). Keep existing assertions for the absence
of the badge and presence of the delete button intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b184446-baec-4b95-ab60-8c6129cd6947
📒 Files selected for processing (27)
docs/features/sidebar-chat-number-shortcuts/plan.mddocs/features/sidebar-chat-number-shortcuts/spec.mddocs/features/sidebar-chat-number-shortcuts/tasks.mdsrc/renderer/src/components/WindowSideBar.vuesrc/renderer/src/components/WindowSideBarSessionItem.vuesrc/renderer/src/i18n/da-DK/thread.jsonsrc/renderer/src/i18n/de-DE/thread.jsonsrc/renderer/src/i18n/en-US/thread.jsonsrc/renderer/src/i18n/es-ES/thread.jsonsrc/renderer/src/i18n/fa-IR/thread.jsonsrc/renderer/src/i18n/fr-FR/thread.jsonsrc/renderer/src/i18n/he-IL/thread.jsonsrc/renderer/src/i18n/id-ID/thread.jsonsrc/renderer/src/i18n/it-IT/thread.jsonsrc/renderer/src/i18n/ja-JP/thread.jsonsrc/renderer/src/i18n/ko-KR/thread.jsonsrc/renderer/src/i18n/ms-MY/thread.jsonsrc/renderer/src/i18n/pl-PL/thread.jsonsrc/renderer/src/i18n/pt-BR/thread.jsonsrc/renderer/src/i18n/ru-RU/thread.jsonsrc/renderer/src/i18n/tr-TR/thread.jsonsrc/renderer/src/i18n/vi-VN/thread.jsonsrc/renderer/src/i18n/zh-CN/thread.jsonsrc/renderer/src/i18n/zh-HK/thread.jsonsrc/renderer/src/i18n/zh-TW/thread.jsontest/renderer/components/WindowSideBar.test.tstest/renderer/components/WindowSideBarSessionItem.test.ts
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
Tests
UI Structure
Before:
After holding Cmd/Alt for 0.5s:
Summary by CodeRabbit