🐛 Fixed search navigating away during CJK IME composition#26878
🐛 Fixed search navigating away during CJK IME composition#26878sailor95 wants to merge 1 commit intoTryGhost:mainfrom
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe change updates keyboard handling in the popup modal component: the handler was renamed from 🚥 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)
📝 Coding Plan
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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/sodo-search/src/components/popup-modal.js (1)
546-551: Consider adding a guard before navigation.If
selectedResultDataor itsurlis undefined (e.g., during a race condition when results update), this would navigate to "undefined". This is pre-existing behavior but could be hardened:🛡️ Proposed defensive guard
if (event.key === 'Enter') { const selectedResultData = allResults.find((d) => { return d.id === selectedResult; }); - window.location.href = selectedResultData?.url; + if (selectedResultData?.url) { + window.location.href = selectedResultData.url; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/sodo-search/src/components/popup-modal.js` around lines 546 - 551, Add a defensive guard before navigating when Enter is pressed: in the Enter key handler that looks up selectedResultData from allResults using selectedResult, verify that selectedResultData exists and that selectedResultData.url is a non-empty string before assigning window.location.href; if the guard fails, do nothing (or optionally show a fallback/error) to avoid navigating to "undefined". Ensure you update the block containing the Enter key check and references to selectedResultData, allResults, selectedResult, and window.location.href.
🤖 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/sodo-search/src/components/popup-modal.js`:
- Around line 546-551: Add a defensive guard before navigating when Enter is
pressed: in the Enter key handler that looks up selectedResultData from
allResults using selectedResult, verify that selectedResultData exists and that
selectedResultData.url is a non-empty string before assigning
window.location.href; if the guard fails, do nothing (or optionally show a
fallback/error) to avoid navigating to "undefined". Ensure you update the block
containing the Enter key check and references to selectedResultData, allResults,
selectedResult, and window.location.href.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 280466bf-cdb4-43c0-888d-4647c19c6760
📒 Files selected for processing (1)
apps/sodo-search/src/components/popup-modal.js
866ecec to
9f012d5
Compare
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
When typing CJK characters (Chinese, Japanese, Korean) in the search modal, pressing Enter to confirm an IME character selection would incorrectly navigate to the first search result. Switched the keyboard handler from keyup to keydown where event.isComposing reliably reflects the composition state.
9f012d5 to
8ac2531
Compare
When using CJK input methods (Chinese, Japanese, Korean) in the sodo-search modal (Cmd+K), pressing Enter to confirm a character during IME composition incorrectly triggers navigation to the first search result. This makes search unusable for CJK users.
The keyboard handler in the
Resultscomponent usedkeyup, butcompositionendfires beforekeyup, soevent.isComposingis alreadyfalseby then. Switched tokeydownwhereisComposingreliably reflects the composition state.Demo
Before (main branch)
pr-26878.mp4
After (PR branch)
pr-26878-after.mov
Note
Low Risk
Low risk: small change to keyboard event handling in the search results list; main risk is unintended key handling differences between
keyupandkeydownfor navigation/Enter behavior.Overview
Fixes Cmd+K search result navigation for CJK IME users by moving the results keyboard listener from
keyuptokeydownand ignoring events during composition (event.isComposing/ legacykeyCode === 229).This prevents Enter presses used to confirm IME text from triggering immediate navigation to the currently selected search result.
Written by Cursor Bugbot for commit 866ecec. This will update automatically on new commits. Configure here.