-
Notifications
You must be signed in to change notification settings - Fork 106
feature: Add keyboard shortcuts for goto def & find all refs #329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Warning Rate limit exceeded@brendan-kellam has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 9 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughKeyboard shortcut support was added to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SymbolHoverPopup
participant Toast
User->>SymbolHoverPopup: Press alt+shift+f12
SymbolHoverPopup->>SymbolHoverPopup: Check if symbol name exists
alt Symbol name exists
SymbolHoverPopup->>SymbolHoverPopup: Call onFindReferences(symbolName)
end
User->>SymbolHoverPopup: Press alt+f12
SymbolHoverPopup->>SymbolHoverPopup: Check if symbol info and definitions exist
alt No definitions found
SymbolHoverPopup->>Toast: Show "No definition found for this symbol."
else Definitions found
SymbolHoverPopup->>SymbolHoverPopup: Call onGotoDefinition()
end
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
a581359
to
cae52dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (1)
100-109
: Remove debug console.log statement.The hotkey implementation is correct, but there's a debug console.log statement that should be removed before production.
useHotkeys('alt+shift+f12', () => { if (symbolInfo?.symbolName) { - console.log('here!'); onFindReferences(symbolInfo.symbolName); } }, {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CHANGELOG.md
(1 hunks)packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx
(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (1)
packages/web/src/components/hooks/use-toast.ts (2)
useToast
(194-194)toast
(194-194)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (5)
packages/web/src/ee/features/codeNav/components/symbolHoverPopup/index.tsx (4)
11-12
: LGTM! Clean import additions.The new imports for
useHotkeys
anduseToast
are correctly added and align with the implementation below.
31-31
: LGTM! Proper hook initialization.The toast hook is correctly initialized for use in the hotkey handlers.
111-128
: LGTM! Well-implemented hotkey with proper error handling.The
alt+f12
hotkey implementation is excellent:
- Properly checks for symbol existence
- Validates symbol definitions availability
- Shows user-friendly toast notification when no definition is found
- Reuses existing
onGotoDefinition
callback for consistency- Includes appropriate accessibility options
The implementation follows good practices and handles edge cases well.
100-128
:❓ Verification inconclusive
Verify hotkey accessibility and keyboard combinations.
The hotkey implementations look functionally correct. However, please verify:
- These key combinations don't conflict with browser/OS shortcuts on different platforms
- The
enableOnFormTags
andenableOnContentEditable
options are appropriate for the use case- Consider documenting these shortcuts in tooltips as mentioned in the PR objectives
🏁 Script executed:
#!/bin/bash # Description: Check if there are any existing hotkey conflicts or documentation about keyboard shortcuts # Search for other hotkey usages in the codebase to check for conflicts rg "useHotkeys.*alt.*f12" -A 3 # Search for any keyboard shortcut documentation or tooltips rg -i "keyboard|shortcut|hotkey|tooltip" --type tsx --type ts -A 2Length of output: 988
Confirm Hotkey Functionality and Documentation
I ran a search across the repo and found only the two bindings in
symbolHoverPopup/index.tsx
. There are no otheralt+f12
oralt+shift+f12
handlers, nor any existing tooltip or shortcut‐documentation references.• Please verify these combinations don’t collide with built-in browser/OS shortcuts on Windows, macOS, and Linux.
• Ensure theenableOnFormTags
andenableOnContentEditable
flags match the intended UX (e.g. do you want these keys active inside inputs?).
• Add in-app tooltips or update your docs/README to surface these new shortcuts to users.CHANGELOG.md (1)
13-13
: LGTM! Accurate changelog entry.The changelog entry correctly documents the new keyboard shortcuts feature and references the appropriate PR number. The entry follows the established format and is placed in the correct section.
This PR adds keyboard shortcuts for code nav:
⌥+f12
-> go to definition⌥+shift+f12
-> find all referencesUnfortunately seems like f12 can't be used since the dev tools panel is binded to f12.
Summary by CodeRabbit
New Features
Documentation