feat: add "By Key" shortcut reverse-lookup filter#852
Merged
pedramamini merged 5 commits intoRunMaestro:rcfrom Apr 23, 2026
Merged
feat: add "By Key" shortcut reverse-lookup filter#852pedramamini merged 5 commits intoRunMaestro:rcfrom
pedramamini merged 5 commits intoRunMaestro:rcfrom
Conversation
Adds a key-capture button next to the filter input in both the Shortcuts settings tab and the keyboard shortcuts help modal. Users can press a key combination to find which action it maps to, rather than searching by name. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…badges Pulls the scrollable list's right edge out by the modal's p-6 padding so the scrollbar sits flush with the modal's right border. Adds gap-4 between each row's label and key badge for better visual separation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughShortcuts UI gains a “By Key” filter mode that captures key sequences, centralized key normalization via new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR adds a "By Key" reverse-lookup filter to both the shortcuts help modal (
Confidence Score: 4/5Safe to merge after fixing the stuck recording-state bug in both files' text input onChange handlers. One P1 defect is present in both changed files: the "By Key" button can get visually stuck in recording mode when the user clicks away to the text input. The one-line fix is straightforward ( Both files need the one-line fix in their text input onChange handlers. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([User opens Shortcuts UI]) --> B{Click 'By Key' button}
B --> C[recordingFilterShortcut = true\nButton focused]
C --> D{User action?}
D -->|Press modifier+key| E[handleFilterRecord captures keys\nfilterShortcutKeys set\nrecordingFilterShortcut = false]
D -->|Press Escape| F[filterShortcutKeys cleared\nrecordingFilterShortcut = false]
D -->|Click text input| G[Button loses focus\nrecordingFilterShortcut stays TRUE ⚠️]
G --> H[User types in text input\nonChange: filterShortcutKeys cleared\nbut recordingFilterShortcut still TRUE]
H --> I[Button stuck showing 'Press keys...'\nno key capture happening]
E --> J[List filters to matching shortcuts]
F --> K[Recording cancelled, list unfiltered]
I --> L{User clicks 'By Key' again}
L --> C
Reviews (1): Last reviewed commit: "fix(shortcuts-help): align scrollbar to ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/renderer/components/Settings/tabs/ShortcutsTab.tsx (2)
93-124: Extract the key-normalization logic into a shared helper.Lines 103-120 duplicate the modifier-capture + Alt/letter+digit normalization block from
handleRecord(lines 56-77), and the entirehandleFilterRecordis duplicated verbatim inShortcutsHelpModal.tsx(lines 60-91). That's three copies of the same normalization. A future fix (e.g. adding a key that needs special handling) would have to be applied in all three places. Consider lifting a pure helper such ascaptureKeyFromEvent(e): string[] | nullintosrc/renderer/utils/shortcutFormatter.ts(or a newshortcutCapture.ts) and having both handlers call it.♻️ Sketch of shared helper
// src/renderer/utils/shortcutCapture.ts export function captureKeyFromEvent(e: React.KeyboardEvent): string[] | null { if (['Meta', 'Control', 'Alt', 'Shift'].includes(e.key)) return null; const keys: string[] = []; if (e.metaKey) keys.push('Meta'); if (e.ctrlKey) keys.push('Ctrl'); if (e.altKey) keys.push('Alt'); if (e.shiftKey) keys.push('Shift'); let mainKey = e.key; if (e.altKey && e.code) { if (e.code.startsWith('Key')) mainKey = e.code.replace('Key', '').toLowerCase(); else if (e.code.startsWith('Digit')) mainKey = e.code.replace('Digit', ''); } keys.push(mainKey); return keys; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/Settings/tabs/ShortcutsTab.tsx` around lines 93 - 124, handleFilterRecord duplicates modifier-capture and Alt/letter+digit normalization logic found in handleRecord and ShortcutsHelpModal; extract that pure logic into a shared helper (e.g. captureKeyFromEvent) and call it from handleFilterRecord, handleRecord and the ShortcutsHelpModal handler. Implement captureKeyFromEvent(e: React.KeyboardEvent): string[] | null to return null for lone modifier keys, build the modifiers array from e.metaKey/e.ctrlKey/e.altKey/e.shiftKey, normalize mainKey using e.code for Alt+Key/Digit cases (matching the sketch in the review), and replace the inline blocks in handleFilterRecord, handleRecord and the modal with a single call that sets state based on the helper return value.
208-253: Exit recording on blur to avoid a stuck "Press keys…" state.Because key capture lives on
onKeyDownCapture, it only fires while the button is focused. If the user enters recording mode and then clicks anywhere else in the settings modal, the button stays visually in "Press keys…" (accent ring) state with no way to cancel short of clicking it again. Consider adding anonBlurthat exits recording mode.♻️ Proposed change
onKeyDownCapture={(e) => { if (recordingFilterShortcut) { handleFilterRecord(e); } }} + onBlur={() => { + if (recordingFilterShortcut) setRecordingFilterShortcut(false); + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/Settings/tabs/ShortcutsTab.tsx` around lines 208 - 253, The button can remain stuck in recording state because key capture uses onKeyDownCapture and only fires while focused; add an onBlur handler on the same button to call setRecordingFilterShortcut(false) (and optionally clear filterShortcutKeys if desired) so recordingFilterShortcut is cleared when focus leaves; update the element that uses filterShortcutButtonRef, recordingFilterShortcut, setRecordingFilterShortcut and handleFilterRecord to ensure onBlur exits recording mode and removes the visual ring.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/Settings/tabs/ShortcutsTab.tsx`:
- Around line 131-138: filteredShortcuts fails to match shortcuts because
captured modifier names (in handleFilterRecord) use "Ctrl" while
TERMINAL_SHORTCUTS uses "Control"; update handleFilterRecord to normalize
captured keys to the canonical names used in definitions (or vice‑versa) before
building filterShortcutKeys so the equality check in filteredShortcuts (which
sorts and joins keys) can succeed—ensure the normalization mapping (e.g., "Ctrl"
<-> "Control", "Alt"/"Option", etc.) is applied consistently to keys produced by
handleFilterRecord and any stored shortcut keys referenced by
TERMINAL_SHORTCUTS.
In `@src/renderer/components/ShortcutsHelpModal.tsx`:
- Around line 159-163: The modal is closing on Escape before the button's
onKeyDownCapture/handleFilterRecord can run because the window-level capture
stops propagation; to fix, add an onBeforeClose handler in the Modal's
layerOptions that prevents closure when recordingFilterShortcut is true (i.e.,
return false or call e.preventDefault) so the layer stack won't close the modal
while the ShortcutsHelpModal is recording; locate the Modal usage in
ShortcutsHelpModal.tsx and update its layerOptions to include onBeforeClose that
checks the recordingFilterShortcut flag and blocks close when active.
---
Nitpick comments:
In `@src/renderer/components/Settings/tabs/ShortcutsTab.tsx`:
- Around line 93-124: handleFilterRecord duplicates modifier-capture and
Alt/letter+digit normalization logic found in handleRecord and
ShortcutsHelpModal; extract that pure logic into a shared helper (e.g.
captureKeyFromEvent) and call it from handleFilterRecord, handleRecord and the
ShortcutsHelpModal handler. Implement captureKeyFromEvent(e:
React.KeyboardEvent): string[] | null to return null for lone modifier keys,
build the modifiers array from e.metaKey/e.ctrlKey/e.altKey/e.shiftKey,
normalize mainKey using e.code for Alt+Key/Digit cases (matching the sketch in
the review), and replace the inline blocks in handleFilterRecord, handleRecord
and the modal with a single call that sets state based on the helper return
value.
- Around line 208-253: The button can remain stuck in recording state because
key capture uses onKeyDownCapture and only fires while focused; add an onBlur
handler on the same button to call setRecordingFilterShortcut(false) (and
optionally clear filterShortcutKeys if desired) so recordingFilterShortcut is
cleared when focus leaves; update the element that uses filterShortcutButtonRef,
recordingFilterShortcut, setRecordingFilterShortcut and handleFilterRecord to
ensure onBlur exits recording mode and removes the visual ring.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b52b2aaf-99ef-405d-8dcd-685c4ea82b2f
📒 Files selected for processing (2)
src/renderer/components/Settings/tabs/ShortcutsTab.tsxsrc/renderer/components/ShortcutsHelpModal.tsx
…ing edge cases - Extract buildKeysFromEvent() into src/renderer/utils/shortcutRecorder.ts and reuse across handleRecord/handleFilterRecord in both ShortcutsTab and ShortcutsHelpModal (previously three near-identical copies). - Cancel "By Key" recording on button blur so it doesn't get stuck when focus moves to the search input. - In ShortcutsHelpModal, add onBeforeClose guard so Escape while recording cancels the recording instead of closing the modal (the window-level layer stack capture-phase listener was preempting the button's onKeyDownCapture. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> EOF )
Commit f0cf7c7 already moved `clearTerminal` out of the "never-rendered TERMINAL_SHORTCUTS group" into DEFAULT_SHORTCUTS but left `newTerminalTab` behind. The remaining export is unused — its one entry isn't merged into `allShortcuts` by the help modal or Settings, so it was never displayed or rebindable. The actual Ctrl+Shift+` handler in useMainKeyboardHandler uses a hardcoded e.code check and doesn't reference this definition. Also eliminates the only 'Control' modifier literal (vs 'Ctrl' used everywhere else), removing a latent filter-mismatch trap. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/utils/shortcutRecorder.ts`:
- Around line 7-26: The alt-key e.code fallback currently in buildKeysFromEvent
runs on all platforms but the comment says it's macOS-only; change the
conditional so the e.code remapping only runs on macOS (e.g., guard with an
isMac() helper or check navigator.platform) so AltGr behavior on Windows/Linux
isn't overridden, and update the JSDoc if you intentionally want cross-platform
behavior; specifically modify the block that checks "if (e.altKey && e.code) {
... }" to "if (isMac() && e.altKey && e.code) { ... }" (or conversely revise the
comment to reflect cross-platform semantics).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6454dfa-95eb-4c20-8192-9806675cb62f
📒 Files selected for processing (4)
src/renderer/components/Settings/tabs/ShortcutsTab.tsxsrc/renderer/components/ShortcutsHelpModal.tsxsrc/renderer/constants/shortcuts.tssrc/renderer/utils/shortcutRecorder.ts
💤 Files with no reviewable changes (1)
- src/renderer/constants/shortcuts.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/components/Settings/tabs/ShortcutsTab.tsx
- src/renderer/components/ShortcutsHelpModal.tsx
…acOS-only The fallback runs unconditionally on every platform — it recovers the physical key whenever Alt rewrites e.key, which happens on macOS Alt-chars (π, ¬, †) and Windows/Linux AltGr layouts alike. Recording and matching must stay symmetric, so comments now describe the actual behavior and call out the symmetry constraint between buildKeysFromEvent and isShortcut. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Collaborator
|
@scriptease thanks for the contribution — nice UX addition! I walked through the recent review round and everything looks buttoned up:
Mergeable is CLEAN, CI is green. Approving — thanks again! |
pedramamini
added a commit
that referenced
this pull request
Apr 23, 2026
- scriptease's PR adds a "By Key" filter button in the Shortcuts Help
modal and the Settings → Shortcuts tab, letting users press a combo
to find which action it maps to. Also extracts a shared
buildKeysFromEvent helper, retires the dead TERMINAL_SHORTCUTS
export, and polishes scrollbar/spacing in the help modal.
- Follow-up commit extracts the duplicated "By Key" button UI into a
shared ui/ShortcutFilterButton component, removes an orphan
trackShortcut('newTerminalTab') call in the main keyboard handler,
and adds unit coverage for buildKeysFromEvent.
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
3 participants
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
Adds a "By Key" filter button next to the existing text filter in both the
keyboard shortcuts overview modal (
⌘/) and the Settings → Shortcuts tab.Users can press any key combination to find which action it's mapped to,
making it easy to discover conflicts or recall forgotten bindings.
to shortcuts matching those exact keys (sorted-set comparison)
filter and vice versa
the scrollbar flush to the modal's right edge; widened row spacing so key
badges don't crowd labels
Screenshots
Shortcuts overview modal —
By Keybutton inactive:Settings → Shortcuts —
By Keyfilter active (recorded combo shown):Test plan
⌘/), click "By Key", press⌘K→ list narrows to Clear TerminalEscwhile recording → cancels without filteringSummary by CodeRabbit
New Features
Bug Fixes
Chores