mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
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 <bhwoo48@gmail.com>
This commit is contained in:
@@ -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(
|
||||
<RichMarkdownSearchBar
|
||||
activeMatchIndex={0}
|
||||
isOpen
|
||||
isReplaceMode
|
||||
matchCase={false}
|
||||
matchCount={2}
|
||||
query={QUERY}
|
||||
replaceQuery={REPLACEMENT}
|
||||
replaceDisabled={false}
|
||||
searchInputRef={createRef<HTMLInputElement>()}
|
||||
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)
|
||||
})
|
||||
})
|
||||
@@ -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()
|
||||
|
||||
+112
@@ -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: () => <span />
|
||||
}))
|
||||
|
||||
vi.mock('@/components/ui/tooltip', () => ({
|
||||
Tooltip: ({ children }: { children?: React.ReactNode }) => <>{children}</>,
|
||||
TooltipContent: ({ children }: { children?: React.ReactNode }) => <div>{children}</div>,
|
||||
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(
|
||||
<RightPanelCommentComposer
|
||||
placeholder="댓글을 입력하세요"
|
||||
submitLabel="등록"
|
||||
onSubmit={onSubmit}
|
||||
/>
|
||||
)
|
||||
})
|
||||
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('확인했습니다')
|
||||
})
|
||||
})
|
||||
@@ -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<HTMLTextAreaElement>) => {
|
||||
// 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()
|
||||
|
||||
Reference in New Issue
Block a user