Fix Ctrl+K shortcut conflict between command menu and link insertion #15163
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR fixes a keyboard shortcut conflict where Ctrl+K/Cmd+K was triggering both the global command menu and the Notes editor's link insertion functionality. The fix adds priority handling in the useCommandMenuHotKeys hook by checking if the active element is within an editor context (identified by the .editor CSS class). When inside an editor, the global command menu shortcut is bypassed, allowing the editor's link shortcut to work correctly.
The implementation uses DOM traversal to detect editor context rather than React state management, suggesting the editor might be a third-party component. This ensures proper separation of concerns between global shortcuts and editor-specific functionality.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-front/src/modules/command-menu/hooks/useCommandMenuHotKeys.ts | 4/5 | Added DOM-based editor context detection to prevent command menu shortcuts from interfering with editor link shortcuts |
Confidence score: 4/5
- This PR is safe to merge with low risk - it addresses a specific UX issue without breaking existing functionality
- Score reflects solid implementation with proper event handling and fallback behavior, though DOM traversal approach could be more robust
- Pay attention to the DOM traversal logic for editor detection - ensure the
.editorclass selector remains consistent across editor implementations
Sequence Diagram
sequenceDiagram
participant User
participant Browser
participant useCommandMenuHotKeys
participant CommandMenu
participant KeyboardShortcutMenu
participant RecordsSearch
participant AISearch
participant Editor
User->>Browser: "Press Ctrl+K/Cmd+K"
Browser->>useCommandMenuHotKeys: "Trigger hotkey callback"
useCommandMenuHotKeys->>Editor: "Check if active element is in editor"
alt Active element is in editor
Editor-->>useCommandMenuHotKeys: "Return true (allow default behavior)"
useCommandMenuHotKeys-->>Browser: "Do not prevent default"
Browser->>Editor: "Execute editor's link shortcut"
else Active element is not in editor
useCommandMenuHotKeys->>Browser: "preventDefault()"
useCommandMenuHotKeys->>KeyboardShortcutMenu: "closeKeyboardShortcutMenu()"
useCommandMenuHotKeys->>CommandMenu: "toggleCommandMenu()"
end
User->>Browser: "Press /"
Browser->>useCommandMenuHotKeys: "Trigger hotkey callback"
useCommandMenuHotKeys->>RecordsSearch: "openRecordsSearchPage()"
User->>Browser: "Press @"
Browser->>useCommandMenuHotKeys: "Trigger hotkey callback"
alt AI is enabled
useCommandMenuHotKeys->>AISearch: "openAskAIPage()"
else AI is disabled
useCommandMenuHotKeys-->>Browser: "Do nothing"
end
User->>Browser: "Press Escape (in command menu)"
Browser->>useCommandMenuHotKeys: "Trigger hotkey callback"
useCommandMenuHotKeys->>CommandMenu: "goBackFromCommandMenu()"
User->>Browser: "Press Backspace/Delete (in command menu)"
Browser->>useCommandMenuHotKeys: "Trigger hotkey callback"
alt Search is not empty
useCommandMenuHotKeys-->>Browser: "Return (do nothing)"
else Search is empty and on root page
useCommandMenuHotKeys->>CommandMenu: "setGlobalCommandMenuContext()"
else Search is empty and not on root page
useCommandMenuHotKeys->>CommandMenu: "goBackFromCommandMenu()"
end
1 file reviewed, 2 comments
| return true; | ||
| } | ||
|
|
||
| if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'k') { |
There was a problem hiding this comment.
style: Redundant condition check - the keys array already defines 'ctrl+k' and 'meta+k', so this manual key check is unnecessary
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/command-menu/hooks/useCommandMenuHotKeys.ts
Line: 55:55
Comment:
**style:** Redundant condition check - the keys array already defines 'ctrl+k' and 'meta+k', so this manual key check is unnecessary
How can I resolve this? If you propose a fix, please make it concise.| const active = document.activeElement; | ||
| const isInEditor = active?.closest('.editor'); | ||
| if (isInEditor !== null) { | ||
| return true; |
There was a problem hiding this comment.
style: Consider returning early with return instead of return true for consistency with other early returns in the codebase
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/command-menu/hooks/useCommandMenuHotKeys.ts
Line: 52:52
Comment:
**style:** Consider returning early with `return` instead of `return true` for consistency with other early returns in the codebase
How can I resolve this? If you propose a fix, please make it concise.
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
| const active = document.activeElement; | ||
| const isInEditor = active?.closest('.editor'); | ||
| if (isInEditor !== null) { | ||
| return true; | ||
| } | ||
|
|
||
| if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'k') { | ||
| e.preventDefault(); | ||
| closeKeyboardShortcutMenu(); | ||
| toggleCommandMenu(); | ||
| } |
There was a problem hiding this comment.
Potential bug: The useGlobalHotkeys hook prevents the default event action before the callback runs, blocking the editor's native ctrl+k shortcut from triggering.
-
Description: The
useGlobalHotkeyshook for thectrl+kshortcut is called without thepreventDefault: falseoption. The hook's implementation stops the keyboard event's propagation and prevents its default action before the provided callback is executed. The callback logic, which checks if the user is inside an editor, runs too late to allow the event to proceed. As a consequence, the editor's native link insertion shortcut (ctrl+k) will not function, as the event is intercepted and stopped before it can reach the editor. -
Suggested fix: Pass an options object with
preventDefault: falseto theuseGlobalHotkeyshook. This ensures the callback executes before the event is handled. Then, inside the callback, manually calle.preventDefault()only when the condition to open the command menu is met (i.e., when not focused within the editor).
severity: 0.65, confidence: 1.0
Did we get this right? 👍 / 👎 to inform future reviews.
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:56415 This environment will automatically shut down when the PR is closed or after 5 hours. |
|
Hello @RajdeepDs, thank you for your contribution, but there's a much simpler way to fix this, I've raised a PR if you want to give it a look #15257 |
3 participants
Description
Ensures the Notes editor’s link shortcut (ctrl+K / cmd+K) takes priority when focused, preventing the global command menu from triggering inside the editor.
Closes #14949