Fix rename inputs committing on CJK IME Enter confirmation (#6980)#7081
Conversation
…6980) Worktree/workspace inline rename, tab rename, editor file-tab rename, and terminal pane title rename all committed whenever `event.key === 'Enter'`, without guarding IME composition state. Pressing Enter to confirm a CJK conversion candidate therefore submitted the rename mid-composition with a half-converted value. Extract the existing local `isComposing || keyCode === 229` guard from WorkspaceDirectorySetting into a shared `isImeCompositionKeyDown` helper and apply it to every rename Enter handler so a composition-confirming Enter no longer commits; a subsequent non-composition Enter does. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
📝 WalkthroughWalkthroughThis PR adds a shared Related issues: Not specified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8daa21a0-d486-46eb-8202-6c048808249c
📒 Files selected for processing (7)
src/renderer/src/components/settings/WorkspaceDirectorySetting.tsxsrc/renderer/src/components/sidebar/WorktreeTitleInlineRename.tsxsrc/renderer/src/components/tab-bar/EditorFileTab.tsxsrc/renderer/src/components/tab-bar/SortableTab.tsxsrc/renderer/src/components/terminal-pane/TerminalPaneHeaderOverlay.tsxsrc/renderer/src/lib/ime-composition-keyboard-event.test.tssrc/renderer/src/lib/ime-composition-keyboard-event.ts
Matches the comment present on every other rename Enter handler in this cohort; CodeRabbit flagged this call site as the only adopter missing it. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-authored-by: Orca <[email protected]>
Jinwoo-H
left a comment
There was a problem hiding this comment.
Thanks for the focused fix. I reviewed the IME guard path and added call-site regression coverage so the actual rename handlers fail if the guard is removed.
Validated:
- CJK composition Enter is ignored on the renamed surfaces, while normal Enter still commits.
- Added regression tests for worktree/workspace title, terminal tab, editor file tab, and terminal pane title rename.
- pnpm typecheck, pnpm lint, affected Vitest files, changed react-doctor lint, and Electron validation passed.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/src/components/sidebar/WorktreeTitleInlineRename.begin-editing.test.tsx (1)
124-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared
pressInputKeytest helper.This helper is duplicated near-identically in
EditorFileTab.test.tsx(lines 331-351) andSortableTab.rename-shortcut.test.tsx(lines 289-307, minusstopPropagation), and re-implemented with a different (real DOMKeyboardEvent) approach inTerminalPaneHeaderOverlay.test.tsx(lines 103-127). The copies have already diverged (some returnstopPropagation, some don't), which risks further drift in this IME-composition-critical test coverage. Consider moving this into a shared test utility (e.g., apress-input-keyhelper module) reused by all four files.♻️ Example extraction
// src/renderer/src/lib/test-utils/press-input-key.ts export function pressInputKey( input: ReactElementLike, key: string, options?: { isComposing?: boolean; keyCode?: number } ): { preventDefault: ReturnType<typeof vi.fn> stopPropagation: ReturnType<typeof vi.fn> } { const event = { key, nativeEvent: { isComposing: options?.isComposing ?? false, keyCode: options?.keyCode ?? 13 }, preventDefault: vi.fn(), stopPropagation: vi.fn() } ;(input.props.onKeyDown as (nextEvent: typeof event) => void)(event) return event }src/renderer/src/components/tab-bar/EditorFileTab.test.tsx (1)
331-351: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
pressInputKeyhelper across test files.This helper is duplicated verbatim (same signature, same event shape) in
SortableTab.rename-shortcut.test.tsxandWorktreeTitleInlineRename.begin-editing.test.tsx. Consider extracting a shared test utility (e.g.src/renderer/src/lib/test-utils/press-input-key.ts) that all three consume, reducing drift risk if the IME contract changes again.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: abe260b7-4023-4045-9790-ec97f0346ffe
📒 Files selected for processing (4)
src/renderer/src/components/sidebar/WorktreeTitleInlineRename.begin-editing.test.tsxsrc/renderer/src/components/tab-bar/EditorFileTab.test.tsxsrc/renderer/src/components/tab-bar/SortableTab.rename-shortcut.test.tsxsrc/renderer/src/components/terminal-pane/TerminalPaneHeaderOverlay.test.tsx
Summary
Fixes #6980.
Worktree/workspace inline rename, tab rename, editor file-tab rename, and terminal pane title rename all committed whenever
event.key === 'Enter', without guarding IME composition state. Pressing Enter to confirm a CJK (Japanese/Chinese/Korean) conversion candidate therefore submitted the rename mid-composition with a half-converted value — the same class of bug as #742, but on rename surfaces the earlier fixes didn't cover.Changes
isComposing || keyCode === 229guard (previously private toWorkspaceDirectorySetting) into a shared, reusable helpersrc/renderer/src/lib/ime-composition-keyboard-event.ts(isImeCompositionKeyDown), with a unit test.Enterhandler that lacked it:WorktreeTitleInlineRename.tsx— worktree card inline rename and workspace rename (⌘⇧R, which opens this same editor)SortableTab.tsx— tab rename (⌘R)TerminalPaneHeaderOverlay.tsx— terminal pane title renameEditorFileTab.tsx— editor file-tab renameWorkspaceDirectorySetting.tsxto consume the shared helper (removes the duplicated local copy).The
keyCode === 229fallback is kept for IMEs/browsers that don't setisComposingon keydown, matching the reference in the issue.Behavior
Testing
vitest— newime-composition-keyboard-event.test.tsplus existingWorkspaceDirectorySetting,WorktreeTitleInlineRename, andSortableTabrename tests pass.oxlint(incl. react-doctor) and webtsgotypecheck pass.🤖 Generated with Claude Code