fix(desktop): prevent keyboard shortcuts from leaking characters into chat input#3520
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 fixes a character-leakage bug in the desktop app where keyboard shortcut combos that produce special characters (e.g. Option+J → Key changes:
Confidence Score: 5/5Safe to merge — minimal, targeted one-line fix with no regressions. The change is a single No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts | Added e.preventDefault() before the user callback in the central hotkey wrapper — correct, minimal fix that prevents character leakage for all registered shortcuts. |
Sequence Diagram
sequenceDiagram
participant User
participant Browser as Browser/Electron
participant useHotkeys as react-hotkeys-hook
participant useHotkey as useHotkey wrapper
participant Callback as Hotkey Callback
participant ChatInput as Chat Input (form/contentEditable)
User->>Browser: Press Option+J (macOS)
Browser->>useHotkeys: keydown event (enableOnFormTags: true)
useHotkeys->>useHotkey: matched hotkey handler(e)
useHotkey->>Browser: e.preventDefault()
Note over Browser,ChatInput: Character insertion blocked
useHotkey->>Callback: callbackRef.current(e)
Callback->>Browser: Switch workspace / perform action
Note over ChatInput: No stray character inserted
Reviews (1): Last reviewed commit: "fix(desktop): prevent default on hotkeys..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts (1)
19-22: UnconditionalpreventDefault()— confirm this is the intended global policy.Calling
e.preventDefault()on every hotkey match fixes the character-leak bug, but changes behavior across alluseHotkeycall sites. Callers inTasksTopBar.tsx,useChatMessageSearch.ts,TerminalPane.tsx, andChatShortcuts.tsxthat explicitly pass{ preventDefault: true }now have redundant options—the field is effectively ignored since you call it unconditionally.Consider either:
- Removing the redundant
preventDefault: trueflags at call sites, or- Making preventDefault opt-out-able with
options?.preventDefault !== falseto preserve flexibility for future hotkeys that may need to allow default behavior (e.g., bare keys like Enter, Tab, or/).Since all registered hotkeys currently use modifier combinations, there's no immediate regression, but this unconditional approach locks in the policy globally.
♻️ Optional: make preventDefault opt-out-able
- (e, _h) => { - e.preventDefault(); - callbackRef.current(e); - }, - { enableOnFormTags: true, enableOnContentEditable: true, ...options }, + (e, _h) => { + if (options?.preventDefault !== false) { + e.preventDefault(); + } + callbackRef.current(e); + }, + { + enableOnFormTags: true, + enableOnContentEditable: true, + ...options, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts` around lines 19 - 22, The handler in useHotkey currently calls e.preventDefault() unconditionally which ignores callers' options; update the inline handler (the (e, _h) => { ... } in useHotkey.ts) to only call e.preventDefault() when options?.preventDefault !== false (i.e., default to preventing but allow opt-out), so callers that passed preventDefault: true remain compatible and callers can opt out by passing preventDefault: false; ensure callbackRef.current(e) is still invoked and adjust any relevant TypeScript types for the options parameter if needed.
🤖 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/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts`:
- Around line 19-22: The handler in useHotkey currently calls e.preventDefault()
unconditionally which ignores callers' options; update the inline handler (the
(e, _h) => { ... } in useHotkey.ts) to only call e.preventDefault() when
options?.preventDefault !== false (i.e., default to preventing but allow
opt-out), so callers that passed preventDefault: true remain compatible and
callers can opt out by passing preventDefault: false; ensure
callbackRef.current(e) is still invoked and adjust any relevant TypeScript types
for the options parameter if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e1b8385-4603-4c62-812c-d5b33c8dc0e8
📒 Files selected for processing (1)
apps/desktop/src/renderer/hotkeys/hooks/useHotkey/useHotkey.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
2 participants
Summary
useHotkeycalled the handler but never callede.preventDefault(), so the browser still processed the key event and typed the charactere.preventDefault()in the centraluseHotkeywrapper before invoking the callback, covering all registered shortcutsTest plan
Summary by cubic
Prevents hotkey combos from inserting characters into inputs when switching workspaces. Hotkeys now block default key actions so the browser doesn’t type stray characters.
useHotkeyhook, calle.preventDefault()before the callback; this is default-on and can be disabled viaoptions.preventDefault = false.Written for commit 1b95c65. Summary will update on new commits.
Summary by CodeRabbit