Fix: Hotkeys stop working after switching between preview and edit modes #1298 #1300
Conversation
- Restructure useMarkdownHotkeys hook to properly reattach event listeners - Track current textarea element to detect when it changes - Ensure hotkeys work after switching between preview and edit modes
|
@ai-engineer-devansh-singh is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughRefactors event listener management in the hotkeys module by introducing refs to track the active textarea and keyboard handler. Replaces inline keydown binding with two coordinated useEffect hooks to ensure listeners attach and detach properly when switching between preview and edit modes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as Editor Component
participant Handler as Keyboard Handler
participant DOM as DOM Event Listener
Note over Editor,DOM: Old Flow (Buggy)
User->>Editor: Mount component
Editor->>DOM: Attach listener (stale reference)
User->>Editor: Switch to preview
Editor->>Editor: Component unmounts listener
User->>Editor: Back to editing
Editor->>Editor: Remount (but handler stale)
User->>Handler: Press hotkey
Handler-->>User: ❌ No response
Note over Editor,DOM: New Flow (Fixed)
User->>Editor: Mount component
Editor->>Editor: useEffect: Store handler in handlerRef
Editor->>DOM: useEffect: Attach fresh listener
User->>Editor: Switch to preview
Editor->>DOM: Cleanup: Detach listener
User->>Editor: Back to editing
Editor->>Editor: useEffect: Create new handler ref
Editor->>DOM: useEffect: Reattach listener with fresh ref
User->>Handler: Press hotkey
Handler-->>User: ✅ Works correctly
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The change involves understanding ref-driven event listener management and verifying that the two-useEffect pattern properly handles textarea swaps and cleanup. Requires careful inspection of dependency arrays and event handler lifecycle to confirm the bug fix is complete. Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
[pre_merge_check_pass] The pull request implementation directly addresses all coding requirements specified in linked issue #1298. The changes to the useMarkdownHotkeys hook include adding refs to track textarea element changes, implementing proper event listener management that reattaches listeners when the textarea is recreated, and cleaning up old listeners to prevent memory leaks. The PR description confirms that all affected hotkeys listed in the issue (Ctrl+1-6, Ctrl+B, Ctrl+I, Ctrl+Shift+B, Ctrl+S, Ctrl+Shift+C, Ctrl+Shift+., Ctrl+L, Ctrl+Shift+I, Ctrl+U, and Backspace) are working after the fix. Manual testing steps align with the issue's reproduction steps and validation requirements. | ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
markdoc/editor/hotkeys/hotkeys.markdoc.ts (2)
174-198: Consider adding dependency array to prevent running on every render.The useEffect has no dependency array, meaning it executes after every component render. While the early return on line 177 prevents unnecessary listener re-attachment, the comparison logic still runs on every render, which could impact performance if this component re-renders frequently.
Since
textareaRef.currentcan change without triggering a re-render (refs are mutable), this pattern may be intentional. However, consider these alternatives:
- Callback ref pattern: Use a callback ref instead of
useRefto get notified when the textarea element changes.- Custom hook: Create a specialized hook that tracks ref changes more efficiently.
Example callback ref approach:
const attachListener = useCallback((textarea: HTMLTextAreaElement | null) => { // Clean up previous if (currentTextareaRef.current && handlerRef.current) { currentTextareaRef.current.removeEventListener('keydown', handlerRef.current); } // Attach new if (textarea && handlerRef.current) { textarea.addEventListener('keydown', handlerRef.current); currentTextareaRef.current = textarea; } else { currentTextareaRef.current = null; } }, []);Then pass
attachListeneras the ref callback.
174-198: Consider leveraging react-hotkeys-hook for consistency.The codebase already uses
react-hotkeys-hook(imported on line 2, used inshortcuts.markdoc.ts). The library is designed to handle hotkey lifecycle management, including the exact issue this PR addresses (listeners remaining on unmounted elements).Consider refactoring to use
useHotkeysfor these keyboard shortcuts as well, which would:
- Eliminate manual event listener management
- Reduce the risk of lifecycle bugs
- Provide a more consistent API across the codebase
Check if the library supports all the required key combinations:
#!/bin/bash # Check how react-hotkeys-hook is currently used in the codebase rg -n "useHotkeys" --type=ts --type=tsx -C 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
markdoc/editor/hotkeys/hotkeys.markdoc.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
markdoc/editor/hotkeys/hotkeys.markdoc.ts (2)
components/CustomTextareAutosize/CustomTextareaAutosize.tsx (2)
props(13-38)node(16-35)markdoc/editor/shortcuts/shortcuts.markdoc.ts (1)
textareaRef(36-101)
🔇 Additional comments (3)
markdoc/editor/hotkeys/hotkeys.markdoc.ts (3)
1-1: LGTM: Import changes support the fix.Adding
useRefis necessary for the ref-based event listener management pattern introduced in this fix.
53-54: LGTM: Refs enable proper listener lifecycle tracking.The refs correctly track the textarea element and handler function, allowing the event listeners to update when the textarea changes without losing the reference to the current handler.
156-172: LGTM: Handler ref pattern avoids unnecessary listener churn.Storing the handler in a ref allows the event listener to automatically use the latest handler without needing to detach/reattach listeners when
handleHotkeychanges. This is an effective optimization.
2 participants
🐛 Bug Fix
Fixes #1298
📝 Description
This PR resolves the issue where keyboard shortcuts (hotkeys) stop functioning after switching from editing mode to preview mode and back to editing mode in the Markdoc editor.
🔧 Changes Made
useMarkdownHotkeyshook inmarkdoc/editor/hotkeys/hotkeys.markdoc.tscurrentTextareaRefandhandlerRef)🧪 Testing
Manual Testing Steps:
/createor/create/{id})Ctrl+Bfor bold)Affected Hotkeys (All working after fix):
Ctrl+1throughCtrl+6- HeadingsCtrl+B- BoldCtrl+I- ItalicCtrl+Shift+B- Bold ItalicCtrl+S- Code snippetCtrl+Shift+C- Code blockCtrl+Shift+.- Block quoteCtrl+L- LinkCtrl+Shift+I- ImageCtrl+U- URLBackspace- Select previous🔍 Root Cause
The issue occurred because:
💡 Solution
The fix ensures that:
📂 Files Changed
markdoc/editor/hotkeys/hotkeys.markdoc.ts- Main fix implementation🎯 Impact