Skip to content

Fix rename inputs committing on CJK IME Enter confirmation (#6980)#7081

Merged
Jinwoo-H merged 3 commits into
stablyai:mainfrom
xianjianlf2:fix/ime-composing-guard-rename-inputs
Jul 3, 2026
Merged

Fix rename inputs committing on CJK IME Enter confirmation (#6980)#7081
Jinwoo-H merged 3 commits into
stablyai:mainfrom
xianjianlf2:fix/ime-composing-guard-rename-inputs

Conversation

@xianjianlf2

Copy link
Copy Markdown
Contributor

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

  • Extract the existing local isComposing || keyCode === 229 guard (previously private to WorkspaceDirectorySetting) into a shared, reusable helper src/renderer/src/lib/ime-composition-keyboard-event.ts (isImeCompositionKeyDown), with a unit test.
  • Apply the guard to every rename Enter handler 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 rename
    • EditorFileTab.tsx — editor file-tab rename
  • Refactor WorkspaceDirectorySetting.tsx to consume the shared helper (removes the duplicated local copy).

The keyCode === 229 fallback is kept for IMEs/browsers that don't set isComposing on keydown, matching the reference in the issue.

Behavior

  • Enter while IME composition is active → only confirms the conversion candidate; the rename input stays open and nothing is committed.
  • A subsequent, non-composition Enter → commits the rename as before.
  • Non-CJK / non-composition input is unchanged.

Testing

  • vitest — new ime-composition-keyboard-event.test.ts plus existing WorkspaceDirectorySetting, WorktreeTitleInlineRename, and SortableTab rename tests pass.
  • oxlint (incl. react-doctor) and web tsgo typecheck pass.

🤖 Generated with Claude Code

…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]>
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a shared isImeCompositionKeyDown helper that checks isComposing and keyCode: 229, along with unit tests for that behavior. It then uses the helper in five renderer rename/edit handlers to return early during IME composition so Enter does not commit or submit. The WorkspaceDirectorySetting component removes its local composition helper and switches to the shared utility. Tests were added or updated for the affected rename flows.

Related issues: Not specified.
Related PRs: Not specified.
Suggested labels: bug, renderer, i18n
Suggested reviewers: Not specified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers summary, behavior, and testing, but it omits required Screenshots, AI Review Report, Security Audit, and Notes sections. Add the missing template sections, including screenshots or a no-visual-change note, AI review and security audit summaries, and notes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately describes the main change: preventing rename commits on CJK IME Enter confirmation.
Linked Issues check ✅ Passed The PR addresses the IME Enter-confirmation bug on worktree, tab, workspace, and window/title rename inputs, matching #6980's requirements.
Out of Scope Changes check ✅ Passed The changes stay within the IME-guard rename fix scope and add only the shared helper plus regression tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8daa21a0-d486-46eb-8202-6c048808249c

📥 Commits

Reviewing files that changed from the base of the PR and between c29bf6f and 3c28548.

📒 Files selected for processing (7)
  • src/renderer/src/components/settings/WorkspaceDirectorySetting.tsx
  • src/renderer/src/components/sidebar/WorktreeTitleInlineRename.tsx
  • src/renderer/src/components/tab-bar/EditorFileTab.tsx
  • src/renderer/src/components/tab-bar/SortableTab.tsx
  • src/renderer/src/components/terminal-pane/TerminalPaneHeaderOverlay.tsx
  • src/renderer/src/lib/ime-composition-keyboard-event.test.ts
  • src/renderer/src/lib/ime-composition-keyboard-event.ts

Comment thread src/renderer/src/components/settings/WorkspaceDirectorySetting.tsx
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]>

@Jinwoo-H Jinwoo-H left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Jinwoo-H
Jinwoo-H merged commit 2848c5d into stablyai:main Jul 3, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/renderer/src/components/sidebar/WorktreeTitleInlineRename.begin-editing.test.tsx (1)

124-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared pressInputKey test helper.

This helper is duplicated near-identically in EditorFileTab.test.tsx (lines 331-351) and SortableTab.rename-shortcut.test.tsx (lines 289-307, minus stopPropagation), and re-implemented with a different (real DOM KeyboardEvent) approach in TerminalPaneHeaderOverlay.test.tsx (lines 103-127). The copies have already diverged (some return stopPropagation, some don't), which risks further drift in this IME-composition-critical test coverage. Consider moving this into a shared test utility (e.g., a press-input-key helper 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 win

Duplicated pressInputKey helper across test files.

This helper is duplicated verbatim (same signature, same event shape) in SortableTab.rename-shortcut.test.tsx and WorktreeTitleInlineRename.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

📥 Commits

Reviewing files that changed from the base of the PR and between 6a8dad6 and 4a851ce.

📒 Files selected for processing (4)
  • src/renderer/src/components/sidebar/WorktreeTitleInlineRename.begin-editing.test.tsx
  • src/renderer/src/components/tab-bar/EditorFileTab.test.tsx
  • src/renderer/src/components/tab-bar/SortableTab.rename-shortcut.test.tsx
  • src/renderer/src/components/terminal-pane/TerminalPaneHeaderOverlay.test.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Worktree/tab/window rename inputs commit on Japanese IME Enter confirmation (missing isComposing guard)

2 participants