fix(defineShortcuts): ignore key events during IME composition#6659
fix(defineShortcuts): ignore key events during IME composition#6659greymoth-jp wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/composables/defineShortcuts.ts`:
- Around line 111-117: `defineShortcuts` currently only skips keydowns while
`e.isComposing` or `e.keyCode === 229`, which still allows the first keydown
immediately after `compositionend` to trigger shortcuts. Update the IME handling
in `defineShortcuts` to mirror the cooldown logic used by `useIMEGuard`, so the
post-composition confirmation keydown is also suppressed. Use the existing
shortcut keydown handler in `defineShortcuts` and align its guard behavior with
`useIMEGuard`.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4eb44e22-a380-41e1-9f3d-a6cc68c5aabc
📒 Files selected for processing (2)
src/runtime/composables/defineShortcuts.tstest/composables/defineShortcuts.spec.ts
| // Ignore keydowns dispatched during IME composition (for example picking a | ||
| // Japanese or Chinese candidate), otherwise the composed keystrokes can match | ||
| // and fire a shortcut. Same condition as the useIMEGuard composable. | ||
| if (e.isComposing || e.keyCode === 229) { | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== useIMEGuard IME handling =="
sed -n '1,60p' src/runtime/composables/useIMEGuard.ts
echo
echo "== defineShortcuts IME handling =="
sed -n '96,140p' src/runtime/composables/defineShortcuts.ts
echo
echo "== composition-related coverage =="
rg -n "compositionend|isComposing|keyCode === 229|IME composition" src/runtime testRepository: nuxt/ui
Length of output: 4856
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== defineShortcuts IME tests =="
sed -n '440,540p' test/composables/defineShortcuts.spec.ts
echo
echo "== defineShortcuts implementation context =="
sed -n '1,220p' src/runtime/composables/defineShortcuts.ts
echo
echo "== useIMEGuard tests =="
sed -n '1,180p' test/composables/useIMEGuard.spec.ts 2>/dev/null || true
echo
echo "== shortcut event wiring =="
rg -n "defineShortcuts\\(|onKeyDown|keydown|compositionend|useIMEGuard" src/runtime testRepository: nuxt/ui
Length of output: 12305
Mirror the IME cooldown here too. e.isComposing || e.keyCode === 229 still lets the first keydown after compositionend through, so Safari IME-confirmation keys can trigger shortcuts. defineShortcuts should match useIMEGuard and suppress that post-composition keydown as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/runtime/composables/defineShortcuts.ts` around lines 111 - 117,
`defineShortcuts` currently only skips keydowns while `e.isComposing` or
`e.keyCode === 229`, which still allows the first keydown immediately after
`compositionend` to trigger shortcuts. Update the IME handling in
`defineShortcuts` to mirror the cooldown logic used by `useIMEGuard`, so the
post-composition confirmation keydown is also suppressed. Use the existing
shortcut keydown handler in `defineShortcuts` and align its guard behavior with
`useIMEGuard`.
commit: |
1 participant
defineShortcutsattaches akeydownlistener towindowand matches every event against the registered shortcuts. It does not check whether an IME composition is in progress, so the keystrokes used to compose CJK text are treated as shortcut input.While composing (Japanese, Chinese, and so on):
usingInputstays active while an input is focused, so it fires on the keys that build up a candidate, including the Enter that confirms it.chainedInputs, so composing romaji such as "gi" can match a chained shortcut likeg-i.The repo already handles this in
useIMEGuard(used byChatPrompt), which bails onevent.isComposing || event.keyCode === 229. This change applies the same condition at the top of thekeydownhandler, so shortcuts are skipped during composition and work normally once it ends. ThekeyCode === 229fallback is kept to matchuseIMEGuard, sinceisComposingis not reliable during composition in every browser.Tests in
test/composables/defineShortcuts.spec.tscover a single-key shortcut and a chained shortcut staying quiet whileisComposingis set, plus a normal press still firing after composition ends. Removing the guard makes the first two fail.