From 47a53b694bc015a747ce37afbc2a95535a5e24da Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 17 Aug 2026 01:05:35 -0700 Subject: [PATCH] fix(ui): ignore IME composition Enter in the comment composer and replace field (#15057) The keydown that exits a CJK composition carries isComposing: true (UI Events 3.6.5, row 5) and, when the IME is processing key input, keyCode 229 (7.3.1). Two handlers acted on it: - right-panel-comment-composer: Cmd/Ctrl+Enter posted the comment while the last syllable was still composing, so it went out truncated. - RichMarkdownSearchBar: Enter ran replace-current, mutating the document from a keystroke aimed at the candidate list; Escape closed the bar instead of letting the IME cancel the composition. Same in the find field. Guard all three with the existing isImeCompositionKeyDown helper, matching the rename and title inputs already on it. The held modifier does not change ownership: a composing Ctrl+Enter is still the IME's keydown, which is why useImeEnterGestureOwnership also owns it and only lets the post-compositionend redispatch chord through. Co-authored-by: BaeTab --- .../RichMarkdownSearchBar.ime-enter.test.tsx | 210 ++++++++++++++++++ .../editor/RichMarkdownSearchBar.tsx | 13 ++ ...-panel-comment-composer.ime-enter.test.tsx | 112 ++++++++++ .../right-panel-comment-composer.tsx | 6 + 4 files changed, 341 insertions(+) create mode 100644 src/renderer/src/components/editor/RichMarkdownSearchBar.ime-enter.test.tsx create mode 100644 src/renderer/src/components/right-sidebar/right-panel-comment-composer.ime-enter.test.tsx diff --git a/src/renderer/src/components/editor/RichMarkdownSearchBar.ime-enter.test.tsx b/src/renderer/src/components/editor/RichMarkdownSearchBar.ime-enter.test.tsx new file mode 100644 index 00000000000..c67702ad350 --- /dev/null +++ b/src/renderer/src/components/editor/RichMarkdownSearchBar.ime-enter.test.tsx @@ -0,0 +1,210 @@ +// @vitest-environment happy-dom + +import { act, createRef } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { RichMarkdownSearchBar } from './RichMarkdownSearchBar' + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +vi.mock('@/hooks/useShortcutLabel', () => ({ + useOptionalShortcutLabel: () => null +})) + +const QUERY = '배포' +const REPLACEMENT = '릴리스' + +let container: HTMLDivElement +let root: Root + +beforeEach(() => { + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => { + root.unmount() + }) + container.remove() +}) + +type BarSpies = { + onClose?: () => void + onMoveToMatch?: (direction: 1 | -1) => void + onReplaceCurrent?: () => void +} + +function renderBar(spies: BarSpies): { + findInput: HTMLInputElement + replaceInput: HTMLInputElement +} { + act(() => { + root.render( + ()} + wholeWord={false} + onClose={spies.onClose ?? vi.fn()} + onMoveToMatch={spies.onMoveToMatch ?? vi.fn()} + onQueryChange={vi.fn()} + onReplaceAll={vi.fn()} + onReplaceCurrent={spies.onReplaceCurrent ?? vi.fn()} + onReplaceQueryChange={vi.fn()} + onToggleMatchCase={vi.fn()} + onToggleReplaceMode={vi.fn()} + onToggleWholeWord={vi.fn()} + /> + ) + }) + const inputs = [...container.querySelectorAll('input')] + // Why: match on value rather than order so a layout change fails loudly instead of + // silently pointing the assertions at the wrong field. + const findInput = inputs.find((candidate) => candidate.value === QUERY) + const replaceInput = inputs.find((candidate) => candidate.value === REPLACEMENT) + if (!findInput || !replaceInput) { + throw new Error('search bar fields not rendered') + } + return { findInput, replaceInput } +} + +function pressKey( + input: HTMLInputElement, + key: string, + init?: KeyboardEventInit & { keyCode?: number } +): void { + const event = new KeyboardEvent('keydown', { + key, + bubbles: true, + cancelable: true, + ...init + }) + if (init?.keyCode !== undefined) { + Object.defineProperty(event, 'keyCode', { value: init.keyCode }) + } + act(() => { + input.dispatchEvent(event) + }) +} + +describe('RichMarkdownSearchBar replace field IME guard', () => { + it('does not replace on the Enter that commits a CJK composition', () => { + const onReplaceCurrent = vi.fn() + const { replaceInput } = renderBar({ onReplaceCurrent }) + + pressKey(replaceInput, 'Enter', { isComposing: true }) + + expect(onReplaceCurrent).not.toHaveBeenCalled() + }) + + it('does not replace on an Enter the IME reports as keyCode 229', () => { + const onReplaceCurrent = vi.fn() + const { replaceInput } = renderBar({ onReplaceCurrent }) + + pressKey(replaceInput, 'Enter', { keyCode: 229 }) + + expect(onReplaceCurrent).not.toHaveBeenCalled() + }) + + it('does not close the bar on the Escape that cancels a composition', () => { + const onClose = vi.fn() + const { replaceInput } = renderBar({ onClose }) + + pressKey(replaceInput, 'Escape', { isComposing: true }) + + expect(onClose).not.toHaveBeenCalled() + }) + + it('still replaces on a plain Enter', () => { + const onReplaceCurrent = vi.fn() + const { replaceInput } = renderBar({ onReplaceCurrent }) + + pressKey(replaceInput, 'Enter') + + expect(onReplaceCurrent).toHaveBeenCalledTimes(1) + }) + + it('still closes the bar on a plain Escape', () => { + const onClose = vi.fn() + const { replaceInput } = renderBar({ onClose }) + + pressKey(replaceInput, 'Escape') + + expect(onClose).toHaveBeenCalledTimes(1) + }) +}) + +describe('RichMarkdownSearchBar find field IME guard', () => { + it('does not move to the next match on the Enter that commits a composition', () => { + const onMoveToMatch = vi.fn() + const { findInput } = renderBar({ onMoveToMatch }) + + pressKey(findInput, 'Enter', { isComposing: true }) + + expect(onMoveToMatch).not.toHaveBeenCalled() + }) + + it('does not move to the previous match on a composing Shift+Enter', () => { + const onMoveToMatch = vi.fn() + const { findInput } = renderBar({ onMoveToMatch }) + + pressKey(findInput, 'Enter', { isComposing: true, shiftKey: true }) + + expect(onMoveToMatch).not.toHaveBeenCalled() + }) + + it('does not move on an Enter the IME reports as keyCode 229', () => { + const onMoveToMatch = vi.fn() + const { findInput } = renderBar({ onMoveToMatch }) + + pressKey(findInput, 'Enter', { keyCode: 229 }) + + expect(onMoveToMatch).not.toHaveBeenCalled() + }) + + it('does not close the bar on the Escape that cancels a composition', () => { + const onClose = vi.fn() + const { findInput } = renderBar({ onClose }) + + pressKey(findInput, 'Escape', { isComposing: true }) + + expect(onClose).not.toHaveBeenCalled() + }) + + it('still moves to the next match on a plain Enter', () => { + const onMoveToMatch = vi.fn() + const { findInput } = renderBar({ onMoveToMatch }) + + pressKey(findInput, 'Enter') + + expect(onMoveToMatch).toHaveBeenCalledWith(1) + }) + + it('still moves to the previous match on a plain Shift+Enter', () => { + const onMoveToMatch = vi.fn() + const { findInput } = renderBar({ onMoveToMatch }) + + pressKey(findInput, 'Enter', { shiftKey: true }) + + expect(onMoveToMatch).toHaveBeenCalledWith(-1) + }) + + it('still closes the bar on a plain Escape', () => { + const onClose = vi.fn() + const { findInput } = renderBar({ onClose }) + + pressKey(findInput, 'Escape') + + expect(onClose).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/components/editor/RichMarkdownSearchBar.tsx b/src/renderer/src/components/editor/RichMarkdownSearchBar.tsx index 9ea8d17e068..4b8944142ea 100644 --- a/src/renderer/src/components/editor/RichMarkdownSearchBar.tsx +++ b/src/renderer/src/components/editor/RichMarkdownSearchBar.tsx @@ -12,6 +12,7 @@ import { import { Button } from '@/components/ui/button' import { Input } from '@/components/ui/input' import { translate } from '@/i18n/i18n' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { useOptionalShortcutLabel } from '@/hooks/useShortcutLabel' type RichMarkdownSearchBarProps = { @@ -111,6 +112,12 @@ export function RichMarkdownSearchBar({ value={query} onChange={(event) => onQueryChange(event.target.value)} onKeyDown={(event) => { + // Why: mid-composition Enter only confirms the candidate and Escape + // only cancels it, so navigating matches or closing the bar here acts + // on a keystroke that was aimed at the IME. + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'Enter' && event.shiftKey) { event.preventDefault() onMoveToMatch(-1) @@ -247,6 +254,12 @@ export function RichMarkdownSearchBar({ value={replaceQuery} onChange={(event) => onReplaceQueryChange(event.target.value)} onKeyDown={(event) => { + // Why: replacing on the Enter that only confirms a candidate mutates + // the document from a keystroke the user aimed at the IME; Escape + // during composition belongs to the IME's cancel, not to the bar. + if (isImeCompositionKeyDown(event)) { + return + } if (event.key === 'Enter') { event.preventDefault() onReplaceCurrent() diff --git a/src/renderer/src/components/right-sidebar/right-panel-comment-composer.ime-enter.test.tsx b/src/renderer/src/components/right-sidebar/right-panel-comment-composer.ime-enter.test.tsx new file mode 100644 index 00000000000..504f1955eb5 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/right-panel-comment-composer.ime-enter.test.tsx @@ -0,0 +1,112 @@ +// @vitest-environment happy-dom + +import React, { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { RightPanelCommentComposer } from './right-panel-comment-composer' + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +vi.mock('@/components/ShortcutKeyCombo', () => ({ + ShortcutKeyCombo: () => +})) + +vi.mock('@/components/ui/tooltip', () => ({ + Tooltip: ({ children }: { children?: React.ReactNode }) => <>{children}, + TooltipContent: ({ children }: { children?: React.ReactNode }) =>
{children}
, + TooltipTrigger: ({ children }: { children?: React.ReactNode }) => <>{children} +})) + +let container: HTMLDivElement +let root: Root + +beforeEach(() => { + // Why: the composer picks its submit modifier off the user agent; pin the non-Mac + // branch so the test drives Ctrl+Enter regardless of the runner's platform. + vi.spyOn(window.navigator, 'userAgent', 'get').mockReturnValue('X11; Linux x86_64') + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) +}) + +afterEach(() => { + act(() => { + root.unmount() + }) + container.remove() + vi.restoreAllMocks() +}) + +function renderComposer(onSubmit: () => Promise<{ ok: true }>): HTMLTextAreaElement { + act(() => { + root.render( + + ) + }) + const textarea = container.querySelector('textarea') + if (!textarea) { + throw new Error('comment textarea not rendered') + } + act(() => { + Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, 'value')?.set?.call( + textarea, + '확인했습니다' + ) + textarea.dispatchEvent(new Event('input', { bubbles: true })) + }) + return textarea +} + +function pressCtrlEnter( + textarea: HTMLTextAreaElement, + init?: KeyboardEventInit & { keyCode?: number } +): void { + const event = new KeyboardEvent('keydown', { + key: 'Enter', + bubbles: true, + cancelable: true, + ctrlKey: true, + ...init + }) + if (init?.keyCode !== undefined) { + Object.defineProperty(event, 'keyCode', { value: init.keyCode }) + } + act(() => { + textarea.dispatchEvent(event) + }) +} + +describe('RightPanelCommentComposer IME Enter guard', () => { + it('does not submit on the Ctrl+Enter that commits a CJK composition', () => { + const onSubmit = vi.fn(async () => ({ ok: true }) as const) + const textarea = renderComposer(onSubmit) + + pressCtrlEnter(textarea, { isComposing: true }) + + expect(onSubmit).not.toHaveBeenCalled() + }) + + it('does not submit on a Ctrl+Enter the IME reports as keyCode 229', () => { + const onSubmit = vi.fn(async () => ({ ok: true }) as const) + const textarea = renderComposer(onSubmit) + + pressCtrlEnter(textarea, { keyCode: 229 }) + + expect(onSubmit).not.toHaveBeenCalled() + }) + + it('still submits on a plain Ctrl+Enter', () => { + const onSubmit = vi.fn(async () => ({ ok: true }) as const) + const textarea = renderComposer(onSubmit) + + pressCtrlEnter(textarea) + + expect(onSubmit).toHaveBeenCalledWith('확인했습니다') + }) +}) diff --git a/src/renderer/src/components/right-sidebar/right-panel-comment-composer.tsx b/src/renderer/src/components/right-sidebar/right-panel-comment-composer.tsx index a3f1e7bae5a..bb6744525e1 100644 --- a/src/renderer/src/components/right-sidebar/right-panel-comment-composer.tsx +++ b/src/renderer/src/components/right-sidebar/right-panel-comment-composer.tsx @@ -4,6 +4,7 @@ import { Button } from '@/components/ui/button' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { ShortcutKeyCombo } from '@/components/ShortcutKeyCombo' import { cn } from '@/lib/utils' +import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event' import { getCommentBodySubmitState, hasBoundedCommentBodyText @@ -171,6 +172,11 @@ export function RightPanelCommentComposer({ const handleKeyDown = useCallback( (event: React.KeyboardEvent) => { + // Why: the Enter that only confirms a CJK candidate still reports the held + // modifier, so submitting here posts the comment without its last syllable. + if (isImeCompositionKeyDown(event)) { + return + } const modifierPressed = isMac ? event.metaKey : event.ctrlKey if (event.key === 'Enter' && modifierPressed) { event.preventDefault()