fix(ime): close remaining composition guard gaps

This commit is contained in:
Neil
2026-08-02 15:20:50 -07:00
parent c315d732e2
commit bfc9378ab1
31 changed files with 615 additions and 54 deletions
@@ -18,6 +18,7 @@ import {
} from '@/lib/quick-workspace-agent-selection'
import type { LinkedWorkItemSummary } from '@/lib/new-workspace'
import { shouldAllowComposerEnterSubmitTarget } from '@/lib/new-workspace-enter-guard'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
import { isScreenSubmitShortcut } from '@/lib/screen-submit-shortcut'
import type {
TuiAgent,
@@ -247,6 +248,9 @@ function QuickTabBody({
return
}
const onKeyDown = (event: KeyboardEvent): void => {
if (isImeCompositionKeyDown(event)) {
return
}
if (event.key !== 'Enter' && event.key !== 'Escape') {
return
}
@@ -212,6 +212,39 @@ describe('BrowserAddressBar autocomplete preview', () => {
expect(onSubmit).not.toHaveBeenCalled()
})
it('keeps suggestions open for an IME-owned Escape', async () => {
const onNavigate = vi.fn()
const onSubmit = vi.fn()
await act(async () => {
root.render(
<AddressBarHarness initialValue="local" onNavigate={onNavigate} onSubmit={onSubmit} />
)
})
const input = container.querySelector<HTMLInputElement>('input[data-orca-browser-address-bar]')
expect(input).not.toBeNull()
await act(async () => {
input?.focus()
})
await act(async () => {
input?.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true }))
})
await act(async () => {
input?.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, isComposing: true })
)
})
expect(container.querySelector('[data-current-address-value="true"]')?.textContent).toBe(
'http://localhost:3000/review-one'
)
expect(onNavigate).not.toHaveBeenCalled()
expect(onSubmit).not.toHaveBeenCalled()
})
it('dismisses suggestions when focus moves into an Electron webview guest', async () => {
const onNavigate = vi.fn()
const onSubmit = vi.fn()
@@ -4,6 +4,7 @@ import { Globe } from 'lucide-react'
import { Input } from '@/components/ui/input'
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
import { cn } from '@/lib/utils'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
import { useAppStore } from '@/store'
import { DEFAULT_SEARCH_ENGINE, type SearchEngine } from '../../../../shared/browser-url'
import { buildBrowserAddressBarSuggestions } from './browser-address-bar-suggestions'
@@ -229,6 +230,9 @@ export default function BrowserAddressBar({
const handleKeyDown = useCallback(
(event: React.KeyboardEvent<HTMLInputElement>) => {
if (isImeCompositionKeyDown(event)) {
return
}
if (event.key === 'Escape') {
cancelSuggestionPreview()
return
@@ -329,6 +333,9 @@ export default function BrowserAddressBar({
}
const handleEscape = (event: KeyboardEvent): void => {
if (isImeCompositionKeyDown(event)) {
return
}
if (event.key !== 'Escape') {
return
}
@@ -11,6 +11,7 @@ import {
import { Input } from '@/components/ui/input'
import { useShortcutLabel } from '@/hooks/useShortcutLabel'
import { translate } from '@/i18n/i18n'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
import type { OpenFile } from '@/store/slices/editor'
import { CLOSE_ALL_CONTEXT_MENUS_EVENT } from '../tab-bar/SortableTab'
import { useEditorHeaderFileRename } from './editor-header-file-rename'
@@ -96,6 +97,9 @@ export function EditorPanelHeaderPath({
onClick={(event) => event.stopPropagation()}
onDoubleClick={(event) => event.stopPropagation()}
onKeyDown={(event) => {
if (isImeCompositionKeyDown(event)) {
return
}
if (event.key === 'Enter') {
event.preventDefault()
event.stopPropagation()
@@ -28,28 +28,36 @@ const LINK_BUBBLE: LinkBubbleState = {
copyEnabled: true
}
function renderBubble(): { onDismiss: ReturnType<typeof vi.fn> } {
function renderBubble({ isEditing = false }: { isEditing?: boolean } = {}): {
onDismiss: ReturnType<typeof vi.fn>
onEditCancel: ReturnType<typeof vi.fn>
onAncestorKeyDown: ReturnType<typeof vi.fn>
} {
const anchorElement = document.createElement('div')
document.body.append(anchorElement)
const onDismiss = vi.fn()
const onEditCancel = vi.fn()
const onAncestorKeyDown = vi.fn()
render(
<TooltipProvider>
<RichMarkdownLinkBubble
anchorElement={anchorElement}
linkBubble={LINK_BUBBLE}
isEditing={false}
onDismiss={onDismiss}
onSave={vi.fn()}
onRemove={vi.fn()}
onEditStart={vi.fn()}
onEditCancel={vi.fn()}
onOpen={vi.fn()}
onCopy={vi.fn()}
/>
</TooltipProvider>
<div onKeyDown={onAncestorKeyDown}>
<TooltipProvider>
<RichMarkdownLinkBubble
anchorElement={anchorElement}
linkBubble={LINK_BUBBLE}
isEditing={isEditing}
onDismiss={onDismiss}
onSave={vi.fn()}
onRemove={vi.fn()}
onEditStart={vi.fn()}
onEditCancel={onEditCancel}
onOpen={vi.fn()}
onCopy={vi.fn()}
/>
</TooltipProvider>
</div>
)
onDismiss.mockClear()
return { onDismiss }
return { onDismiss, onEditCancel, onAncestorKeyDown }
}
describe('RichMarkdownLinkBubble viewport dismissal', () => {
@@ -80,4 +88,25 @@ describe('RichMarkdownLinkBubble viewport dismissal', () => {
expect(onDismiss).toHaveBeenCalled()
})
it('keeps link editing open for an IME-owned Escape', () => {
const { onDismiss, onEditCancel, onAncestorKeyDown } = renderBubble({ isEditing: true })
const input = document.querySelector<HTMLInputElement>('.rich-markdown-link-input')
fireEvent.keyDown(input!, { key: 'Escape', keyCode: 27, isComposing: true })
expect(onEditCancel).not.toHaveBeenCalled()
expect(onDismiss).not.toHaveBeenCalled()
expect(onAncestorKeyDown).not.toHaveBeenCalled()
})
it('still cancels link editing for ordinary Escape', () => {
const { onDismiss, onEditCancel } = renderBubble({ isEditing: true })
const input = document.querySelector<HTMLInputElement>('.rich-markdown-link-input')
fireEvent.keyDown(input!, { key: 'Escape', keyCode: 27 })
expect(onEditCancel).toHaveBeenCalledOnce()
expect(onDismiss).not.toHaveBeenCalled()
})
})
@@ -6,6 +6,7 @@ import { translate } from '@/i18n/i18n'
import { Button } from '@/components/ui/button'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { addViewportSizeChangeListener } from '@/hooks/viewport-size-change-listener'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
export type LinkBubbleState = {
kind: 'markdown' | 'html-superscript'
@@ -134,6 +135,9 @@ function LinkEditInput({
value={value}
onChange={(e) => setValue(e.target.value)}
onKeyDown={(e) => {
if (isImeCompositionKeyDown(e)) {
return
}
if (e.key === 'Enter') {
e.preventDefault()
onSave(value.trim())
@@ -298,6 +302,9 @@ export function RichMarkdownLinkBubble({
}}
onKeyDown={(event) => {
event.stopPropagation()
if (isImeCompositionKeyDown(event)) {
return
}
if (event.key === 'Escape') {
event.preventDefault()
onDismiss()
@@ -11,6 +11,7 @@ import { TYPE_FIELD_DATA_TYPE } from './columns'
import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
import { Input } from '@/components/ui/input'
import { cn } from '@/lib/utils'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
import { useRepoAssigneesBySlug, useRepoLabelsBySlug } from '@/hooks/useGitHubSlugMetadata'
import { useAppStore } from '@/store'
import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client'
@@ -726,6 +727,9 @@ function TextCell({
}
}}
onKeyDown={(e) => {
if (isImeCompositionKeyDown(e)) {
return
}
if (e.key === 'Enter') {
e.preventDefault()
setEditing(false)
@@ -779,6 +783,9 @@ function DateCell({
}
}}
onKeyDown={(e) => {
if (isImeCompositionKeyDown(e)) {
return
}
if (e.key === 'Enter') {
e.preventDefault()
;(e.target as HTMLInputElement).blur()
@@ -6,6 +6,7 @@ import { ImageIcon, Paperclip } from 'lucide-react'
import { Button } from '@/components/ui/button'
import { Input } from '@/components/ui/input'
import { cn } from '@/lib/utils'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
import { isScreenSubmitShortcut } from '@/lib/screen-submit-shortcut'
import { createRichMarkdownExtensions } from '@/components/editor/rich-markdown-extensions'
import { RichMarkdownToolbar } from '@/components/editor/RichMarkdownToolbar'
@@ -26,26 +27,13 @@ import {
} from '@/components/github/github-markdown-composer-tabbar'
import { hasBoundedGitHubMarkdownImageUrlText } from '@/components/github/github-markdown-image-url'
import { useImageInput } from '@/components/github/use-image-input'
import type { GitHubOwnerRepo } from '../../../../shared/types'
import { translate } from '@/i18n/i18n'
import { useAppStore } from '@/store'
import {
getRichMarkdownSpellcheckAttribute,
useRichMarkdownSpellcheckAttribute
} from '@/components/editor/rich-markdown-spellcheck'
type GitHubMarkdownComposerProps = {
value: string
onChange: (value: string) => void
placeholder: string
minHeightClassName?: string
className?: string
disabled?: boolean
autoFocus?: boolean
onSubmitShortcut?: () => void
layout?: 'stacked' | 'tabbed'
previewGithubRepo?: GitHubOwnerRepo | null
}
import type { GitHubMarkdownComposerProps } from './github-markdown-composer-props'
export function GitHubMarkdownComposer({
value,
@@ -129,6 +117,9 @@ export function GitHubMarkdownComposer({
spellcheck: getRichMarkdownSpellcheckAttribute(richMarkdownSpellcheckEnabled)
},
handleKeyDown: (_view, event) => {
if (isImeCompositionKeyDown(event)) {
return false
}
if (isScreenSubmitShortcut(event)) {
const submit = onSubmitShortcutRef.current
if (submit) {
@@ -306,6 +297,9 @@ export function GitHubMarkdownComposer({
value={imageUrl}
onChange={(event) => setImageUrl(event.target.value)}
onKeyDown={(event) => {
if (isImeCompositionKeyDown(event)) {
return
}
if (isScreenSubmitShortcut(event)) {
event.preventDefault()
event.stopPropagation()
@@ -0,0 +1,14 @@
import type { GitHubOwnerRepo } from '../../../../shared/types'
export type GitHubMarkdownComposerProps = {
value: string
onChange: (value: string) => void
placeholder: string
minHeightClassName?: string
className?: string
disabled?: boolean
autoFocus?: boolean
onSubmitShortcut?: () => void
layout?: 'stacked' | 'tabbed'
previewGithubRepo?: GitHubOwnerRepo | null
}
@@ -0,0 +1,40 @@
// @vitest-environment happy-dom
import { cleanup, renderHook } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { useMobilePageEscape } from './use-mobile-page-escape'
afterEach(() => {
cleanup()
document.body.replaceChildren()
})
describe('useMobilePageEscape', () => {
it('leaves a composing input focused for marked Escape', () => {
const onClose = vi.fn()
const input = document.createElement('input')
document.body.append(input)
renderHook(() => useMobilePageEscape(onClose))
input.focus()
input.dispatchEvent(
new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, isComposing: true })
)
expect(document.activeElement).toBe(input)
expect(onClose).not.toHaveBeenCalled()
})
it('still blurs a focused input for ordinary Escape', () => {
const onClose = vi.fn()
const input = document.createElement('input')
document.body.append(input)
renderHook(() => useMobilePageEscape(onClose))
input.focus()
input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))
expect(document.activeElement).not.toBe(input)
expect(onClose).not.toHaveBeenCalled()
})
})
@@ -1,4 +1,5 @@
import { useEffect } from 'react'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
function isEditableElement(target: EventTarget | null): target is HTMLElement {
return (
@@ -13,6 +14,9 @@ export function useMobilePageEscape(onClose: () => void): void {
// Why: mirror Automations/Tasks — Esc first exits field focus, then closes the page.
useEffect(() => {
function onKeyDown(event: KeyboardEvent): void {
if (isImeCompositionKeyDown(event)) {
return
}
if (event.key !== 'Escape' || event.defaultPrevented) {
return
}
@@ -120,7 +120,7 @@ export default function WorkspaceKanbanSearchField({
// this field is the only handler — it must cover both outcomes.
// Mid-composition Escape belongs to the IME, which cancels the
// in-progress reading rather than the query behind it.
if (event.key !== 'Escape' || isImeCompositionKeyDown(event)) {
if (isImeCompositionKeyDown(event) || event.key !== 'Escape') {
return
}
event.preventDefault()
@@ -6,6 +6,7 @@ import type { PtyTransport } from './pty-transport'
import { useTerminalKeyboardShortcuts } from './keyboard-handlers'
import {
installTerminalImeCompositionRoute,
XTERM_COMPOSITION_SESSION_END_EVENT,
XTERM_COMPOSITION_SESSION_START_EVENT
} from './terminal-ime-composition-route'
@@ -31,6 +32,7 @@ function keyboardEvent(
function createHarness(options: { paneCount?: number } = {}): {
deps: KeyboardHandlersDeps
editable: HTMLInputElement
endComposition: () => void
sendInput: ReturnType<typeof vi.fn>
startComposition: () => void
terminalInput: HTMLTextAreaElement
@@ -103,6 +105,13 @@ function createHarness(options: { paneCount?: number } = {}): {
return {
deps,
editable,
endComposition: () => {
terminalElement.dispatchEvent(
new CustomEvent(XTERM_COMPOSITION_SESSION_END_EVENT, {
detail: { id: 1, data: '한' }
})
)
},
sendInput,
terminalInput,
startComposition: () => {
@@ -326,6 +335,106 @@ describe('Windows IME keyboard ownership', () => {
hook.unmount()
harness.dispose()
})
it('clears held Shift when a consumed keyup reports that the modifier is no longer down', () => {
const harness = createHarness()
const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps))
harness.terminalInput.dispatchEvent(
keyboardEvent('keydown', {
key: 'Shift',
code: 'ShiftLeft',
keyCode: 16,
timeStamp: 1,
shiftKey: true
})
)
harness.startComposition()
harness.terminalInput.dispatchEvent(
keyboardEvent('keyup', {
key: 'Process',
code: 'ShiftLeft',
keyCode: 229,
timeStamp: 10,
isComposing: true
})
)
harness.terminalInput.dispatchEvent(
keyboardEvent('keyup', {
key: 'Enter',
code: 'Enter',
keyCode: 13,
timeStamp: 20,
isComposing: true
})
)
harness.endComposition()
vi.runAllTimers()
expect(harness.sendInput).not.toHaveBeenCalled()
hook.unmount()
harness.dispose()
})
it('uses held Shift for a bare balancing Enter keyup during composition', () => {
const harness = createHarness()
const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps))
harness.terminalInput.dispatchEvent(
keyboardEvent('keydown', {
key: 'Shift',
code: 'ShiftLeft',
keyCode: 16,
timeStamp: 1,
shiftKey: true
})
)
harness.startComposition()
const keyup = keyboardEvent('keyup', {
key: 'Enter',
code: 'Enter',
keyCode: 13,
timeStamp: 20,
isComposing: true
})
harness.terminalInput.dispatchEvent(keyup)
expect(keyup.defaultPrevented).toBe(true)
expect(harness.sendInput).not.toHaveBeenCalled()
harness.endComposition()
vi.runAllTimers()
expect(harness.sendInput).toHaveBeenCalledOnce()
expect(harness.sendInput).toHaveBeenCalledWith('\x1b\r')
hook.unmount()
harness.dispose()
})
it.each([{ code: 'Enter' }, { code: 'NumpadEnter' }])(
'still defers a composed Shift+Enter reported as Process with code $code',
({ code }) => {
const harness = createHarness()
const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps))
harness.startComposition()
const processEnter = keyboardEvent('keydown', {
key: 'Process',
code,
keyCode: 229,
timeStamp: 10,
isComposing: true,
shiftKey: true
})
harness.terminalInput.dispatchEvent(processEnter)
expect(processEnter.defaultPrevented).toBe(true)
expect(harness.sendInput).not.toHaveBeenCalled()
harness.endComposition()
vi.runAllTimers()
expect(harness.sendInput).toHaveBeenCalledWith('\x1b\r')
hook.unmount()
harness.dispose()
}
)
})
describe('terminal shortcuts yield to a composition', () => {
@@ -281,6 +281,25 @@ export function useTerminalKeyboardShortcuts({
const nativeOnlyShortcutTracker = createTerminalNativeOnlyShortcutTracker()
const deferredNewlineSender = createTerminalImeDeferredNewlineSender()
const modifiedEnterChordOwner = createTerminalImeModifiedEnterChordOwner()
const reconcileHeldImeEnterModifiers = (
event: KeyboardEvent,
preserveModifierLostRedispatch = false
): void => {
if (heldImeEnterModifiers.size === 0) {
return
}
for (const [kind, pressed] of [
['shift', event.shiftKey],
['ctrl', event.ctrlKey]
] as const) {
const belongsToActiveChord =
preserveModifierLostRedispatch &&
modifiedEnterChordOwner.absorb({ kind, code: event.code, timeStamp: event.timeStamp })
if (!pressed && !belongsToActiveChord && heldImeEnterModifiers.delete(kind)) {
modifiedEnterChordOwner.release({ kind, code: event.code, timeStamp: event.timeStamp })
}
}
}
const getHeldImeEnterModifier = () =>
heldImeEnterModifiers.size === 1
? (heldImeEnterModifiers.values().next().value ?? null)
@@ -297,6 +316,7 @@ export function useTerminalKeyboardShortcuts({
return kind ? { kind, code: event.code, timeStamp: event.timeStamp } : null
}
const onModifierDown = (e: KeyboardEvent): void => {
reconcileHeldImeEnterModifiers(e, e.key === 'Enter' && e.keyCode === 13)
if (e.key === 'Alt') {
optionKeyLocation = e.location
}
@@ -452,6 +472,7 @@ export function useTerminalKeyboardShortcuts({
deferredNewlineSender.absorbRedispatchedEnter(e))
) {
// Chromium can drop the modifier when re-dispatching the committing Enter.
reconcileHeldImeEnterModifiers(e)
e.preventDefault()
e.stopImmediatePropagation()
return
@@ -749,6 +770,9 @@ export function useTerminalKeyboardShortcuts({
}
const onKeyUp = (e: KeyboardEvent): void => {
if (!isTerminalImeEnterKeyUp(e)) {
reconcileHeldImeEnterModifiers(e)
}
if (e.key === 'Alt') {
optionKeyLocation = 0
}
@@ -475,6 +475,7 @@ describe('isTerminalImeProcessEnter', () => {
const event = (overrides: Partial<KeyboardEvent> = {}) =>
({
key: 'Process',
code: 'Enter',
keyCode: 229,
metaKey: false,
ctrlKey: false,
@@ -483,19 +484,23 @@ describe('isTerminalImeProcessEnter', () => {
...overrides
}) as KeyboardEvent
it.each([{ shiftKey: true }, { shiftKey: false, ctrlKey: true }])(
'recognizes a Windows IME modifier Enter reported as Process',
(modifiers) => {
expect(isTerminalImeProcessEnter(event(modifiers))).toBe(true)
}
)
it.each([
{ shiftKey: true },
{ shiftKey: false, ctrlKey: true },
{ code: 'NumpadEnter' },
{ code: 'NumpadEnter', shiftKey: false, ctrlKey: true }
])('recognizes a Windows IME modifier Enter reported as Process', (modifiers) => {
expect(isTerminalImeProcessEnter(event(modifiers))).toBe(true)
})
it.each([
{ key: 'Enter' },
{ keyCode: 13 },
{ shiftKey: false },
{ ctrlKey: true },
{ altKey: true }
{ altKey: true },
{ code: 'KeyQ' },
{ code: 'KeyI', shiftKey: false, ctrlKey: true }
])('rejects a non-IME or ambiguous Process key', (override) => {
expect(isTerminalImeProcessEnter(event(override))).toBe(false)
})
@@ -165,11 +165,16 @@ export function getTerminalImeModifiedEnterKind(
}
export function isTerminalImeProcessEnter(
event: Pick<KeyboardEvent, 'key' | 'keyCode' | 'metaKey' | 'ctrlKey' | 'altKey' | 'shiftKey'>
event: Pick<
KeyboardEvent,
'key' | 'code' | 'keyCode' | 'metaKey' | 'ctrlKey' | 'altKey' | 'shiftKey'
>
): boolean {
return (
event.key === 'Process' &&
event.keyCode === 229 &&
// Windows reports every IME-consumed key as Process/229; only code proves physical Enter.
(event.code === 'Enter' || event.code === 'NumpadEnter') &&
getTerminalImeModifiedEnterKind(event) !== null
)
}
@@ -33,6 +33,10 @@ describe('terminalShortcutIsOwnedByIme', () => {
// already handles composition, rather than merely declining to fix one.
it.each([
['the Process shape', { key: 'Process', code: 'Enter', keyCode: 229, isComposing: true }],
[
'the numpad Process shape',
{ key: 'Process', code: 'NumpadEnter', keyCode: 229, isComposing: true }
],
['the marked real shape', { key: 'Enter', code: 'Enter', keyCode: 13, isComposing: true }]
])('releases Enter in %s for a caller that defers it to the commit', (_shape, init) => {
expect(
@@ -42,5 +42,6 @@ export function terminalShortcutIsOwnedByIme(
if (action?.type === 'switchInputSource') {
return false
}
return !(enterIsDeferredToCommit && event.code === 'Enter' && action?.type === 'sendInput')
const isPhysicalEnter = event.code === 'Enter' || event.code === 'NumpadEnter'
return !(enterIsDeferredToCommit && isPhysicalEnter && action?.type === 'sendInput')
}
@@ -6,6 +6,7 @@ import { SearchIcon } from 'lucide-react'
import { Dialog as DialogPrimitive } from 'radix-ui'
import { cn } from '@/lib/utils'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function Command({ className, ...props }: React.ComponentProps<typeof CommandPrimitive>) {
return (
@@ -42,6 +43,7 @@ function CommandDialog({
commandProps?: React.ComponentProps<typeof CommandPrimitive>
}) {
const { className: commandClassName, ...commandRootProps } = commandProps ?? {}
const handleEscapeKeyDown = useImeAwareEscapeKeyDown()
return (
<DialogPrimitive.Root {...props}>
@@ -67,6 +69,7 @@ function CommandDialog({
)}
onOpenAutoFocus={onOpenAutoFocus}
onCloseAutoFocus={onCloseAutoFocus}
onEscapeKeyDown={handleEscapeKeyDown}
>
<DialogPrimitive.Title className="sr-only">{title}</DialogPrimitive.Title>
<DialogPrimitive.Description className="sr-only">
@@ -3,6 +3,7 @@ import { CheckIcon, ChevronRightIcon, CircleIcon } from 'lucide-react'
import { ContextMenu as ContextMenuPrimitive } from 'radix-ui'
import { cn } from '@/lib/utils'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function ContextMenu({ ...props }: React.ComponentProps<typeof ContextMenuPrimitive.Root>) {
return <ContextMenuPrimitive.Root data-slot="context-menu" modal={false} {...props} />
@@ -59,8 +60,11 @@ function ContextMenuSubTrigger({
function ContextMenuSubContent({
className,
style,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof ContextMenuPrimitive.SubContent>) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<ContextMenuPrimitive.Portal>
<ContextMenuPrimitive.SubContent
@@ -73,6 +77,7 @@ function ContextMenuSubContent({
// overflow clipping does not hide the cascade on click/hover.
style={{ ...style, WebkitAppRegion: 'no-drag' } as React.CSSProperties}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
/>
</ContextMenuPrimitive.Portal>
)
@@ -81,8 +86,11 @@ function ContextMenuSubContent({
function ContextMenuContent({
className,
style,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof ContextMenuPrimitive.Content>) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<ContextMenuPrimitive.Portal>
<ContextMenuPrimitive.Content
@@ -95,6 +103,7 @@ function ContextMenuContent({
// capture clicks at the OS level when menus overlap them.
style={{ ...style, WebkitAppRegion: 'no-drag' } as React.CSSProperties}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
/>
</ContextMenuPrimitive.Portal>
)
@@ -7,6 +7,7 @@ import { Dialog as DialogPrimitive } from 'radix-ui'
import { cn } from '@/lib/utils'
import { Button } from '@/components/ui/button'
import { translate } from '@/i18n/i18n'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function Dialog({ ...props }: React.ComponentProps<typeof DialogPrimitive.Root>) {
return <DialogPrimitive.Root data-slot="dialog" {...props} />
@@ -48,11 +49,14 @@ function DialogContent({
children,
overlayClassName,
showCloseButton = true,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof DialogPrimitive.Content> & {
overlayClassName?: string
showCloseButton?: boolean
}) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<DialogPortal data-slot="dialog-portal">
<DialogOverlay className={overlayClassName} />
@@ -68,6 +72,7 @@ function DialogContent({
className
)}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
>
{children}
{showCloseButton && (
@@ -0,0 +1,50 @@
import { readdirSync, readFileSync } from 'node:fs'
import { basename, join } from 'node:path'
import { describe, expect, it } from 'vitest'
const UI_DIR = 'src/renderer/src/components/ui'
const NON_DISMISSABLE_CONTENT = new Set(['accordion.tsx', 'collapsible.tsx', 'tabs.tsx'])
const MUST_GUARD = [
'command.tsx',
'context-menu.tsx',
'dialog.tsx',
'dropdown-menu.tsx',
'hover-card.tsx',
'popover.tsx',
'select.tsx',
'sheet.tsx',
'tooltip.tsx'
]
function count(source: string, pattern: RegExp): number {
return source.match(pattern)?.length ?? 0
}
describe('Radix dismissable-layer wrappers guard IME Escape', () => {
it('guards every dismissable Primitive.Content exported from the UI layer', () => {
const guarded: string[] = []
const offenders: string[] = []
for (const entry of readdirSync(UI_DIR, { withFileTypes: true })) {
if (!entry.isFile() || !entry.name.endsWith('.tsx') || entry.name.includes('.test.')) {
continue
}
const file = join(UI_DIR, entry.name)
const source = readFileSync(file, 'utf8')
const contentCount = count(source, /<\w+Primitive\.(?:Sub)?Content\b/g)
if (contentCount === 0 || NON_DISMISSABLE_CONTENT.has(entry.name)) {
continue
}
const hookCount = count(source, /const handleEscapeKeyDown = useImeAwareEscapeKeyDown\(/g)
const propCount = count(source, /onEscapeKeyDown=\{handleEscapeKeyDown\}/g)
if (hookCount !== contentCount || propCount !== contentCount) {
offenders.push(basename(file))
} else {
guarded.push(basename(file))
}
}
expect(offenders).toEqual([])
expect(guarded).toEqual(expect.arrayContaining(MUST_GUARD))
})
})
@@ -0,0 +1,130 @@
// @vitest-environment happy-dom
import { cleanup, fireEvent, render, waitFor } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { useState } from 'react'
import { Dialog, DialogContent, DialogTitle } from './dialog'
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuSub,
DropdownMenuSubContent,
DropdownMenuSubTrigger,
DropdownMenuTrigger
} from './dropdown-menu'
import { Popover, PopoverContent, PopoverTrigger } from './popover'
import { Sheet, SheetContent, SheetTitle } from './sheet'
type SurfaceProps = { onEscapeKeyDown: (event: KeyboardEvent) => void }
function ControlledPopover({ onEscapeKeyDown }: SurfaceProps): React.JSX.Element {
const [open, setOpen] = useState(true)
return (
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger>Open</PopoverTrigger>
<PopoverContent onEscapeKeyDown={onEscapeKeyDown}>
<input aria-label="Draft" />
</PopoverContent>
</Popover>
)
}
function ControlledDialog({ onEscapeKeyDown }: SurfaceProps): React.JSX.Element {
const [open, setOpen] = useState(true)
return (
<Dialog open={open} onOpenChange={setOpen}>
<DialogContent onEscapeKeyDown={onEscapeKeyDown}>
<DialogTitle>Draft dialog</DialogTitle>
<input aria-label="Draft" />
</DialogContent>
</Dialog>
)
}
function ControlledSheet({ onEscapeKeyDown }: SurfaceProps): React.JSX.Element {
const [open, setOpen] = useState(true)
return (
<Sheet open={open} onOpenChange={setOpen}>
<SheetContent onEscapeKeyDown={onEscapeKeyDown}>
<SheetTitle>Draft sheet</SheetTitle>
<input aria-label="Draft" />
</SheetContent>
</Sheet>
)
}
function ControlledDropdownSub({ onEscapeKeyDown }: SurfaceProps): React.JSX.Element {
const [open, setOpen] = useState(true)
return (
<DropdownMenu open={open} onOpenChange={setOpen}>
<DropdownMenuTrigger>Open</DropdownMenuTrigger>
<DropdownMenuContent>
<DropdownMenuSub>
<DropdownMenuSubTrigger>More</DropdownMenuSubTrigger>
<DropdownMenuSubContent onEscapeKeyDown={onEscapeKeyDown}>
<input aria-label="Draft" />
</DropdownMenuSubContent>
</DropdownMenuSub>
</DropdownMenuContent>
</DropdownMenu>
)
}
const SURFACES = [
['PopoverContent', ControlledPopover],
['DialogContent', ControlledDialog],
['SheetContent', ControlledSheet]
] as const
afterEach(cleanup)
describe('Radix dismissable-layer IME ownership', () => {
it.each(SURFACES)('%s keeps its draft open for a marked Escape', (_name, Surface) => {
const onEscapeKeyDown = vi.fn()
const view = render(<Surface onEscapeKeyDown={onEscapeKeyDown} />)
const input = view.getByRole('textbox', { name: 'Draft' })
fireEvent.keyDown(input, { key: 'Escape', keyCode: 27, isComposing: true })
expect(view.queryByRole('textbox', { name: 'Draft' })).not.toBeNull()
expect(onEscapeKeyDown).not.toHaveBeenCalled()
})
it.each(SURFACES)('%s still dismisses for an ordinary Escape', (_name, Surface) => {
const onEscapeKeyDown = vi.fn()
const view = render(<Surface onEscapeKeyDown={onEscapeKeyDown} />)
const input = view.getByRole('textbox', { name: 'Draft' })
fireEvent.keyDown(input, { key: 'Escape', keyCode: 27 })
expect(view.queryByRole('textbox', { name: 'Draft' })).toBeNull()
expect(onEscapeKeyDown).toHaveBeenCalledOnce()
})
async function openDropdownSub(onEscapeKeyDown: (event: KeyboardEvent) => void) {
const view = render(<ControlledDropdownSub onEscapeKeyDown={onEscapeKeyDown} />)
fireEvent.keyDown(view.getByRole('menuitem', { name: 'More' }), { key: 'ArrowRight' })
const input = await view.findByRole('textbox', { name: 'Draft' })
return { input, view }
}
it('keeps DropdownMenuSubContent open for a marked Escape', async () => {
const onEscapeKeyDown = vi.fn()
const { input, view } = await openDropdownSub(onEscapeKeyDown)
fireEvent.keyDown(input, { key: 'Escape', keyCode: 27, isComposing: true })
expect(view.queryByRole('textbox', { name: 'Draft' })).not.toBeNull()
expect(onEscapeKeyDown).not.toHaveBeenCalled()
})
it('still dismisses DropdownMenuSubContent for an ordinary Escape', async () => {
const onEscapeKeyDown = vi.fn()
const { input, view } = await openDropdownSub(onEscapeKeyDown)
fireEvent.keyDown(input, { key: 'Escape', keyCode: 27 })
await waitFor(() => expect(view.queryByRole('textbox', { name: 'Draft' })).toBeNull())
expect(onEscapeKeyDown).toHaveBeenCalledOnce()
})
})
@@ -3,6 +3,7 @@ import { CheckIcon, ChevronRightIcon, CircleIcon } from 'lucide-react'
import { DropdownMenu as DropdownMenuPrimitive } from 'radix-ui'
import { cn } from '@/lib/utils'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function DropdownMenu({ ...props }: React.ComponentProps<typeof DropdownMenuPrimitive.Root>) {
return <DropdownMenuPrimitive.Root data-slot="dropdown-menu" {...props} />
@@ -24,8 +25,11 @@ function DropdownMenuContent({
className,
sideOffset = 4,
style,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof DropdownMenuPrimitive.Content>) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<DropdownMenuPrimitive.Portal>
<DropdownMenuPrimitive.Content
@@ -40,6 +44,7 @@ function DropdownMenuContent({
// dropdown menus that visually overlap the titlebar are unclickable.
style={{ ...style, WebkitAppRegion: 'no-drag' } as React.CSSProperties}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
/>
</DropdownMenuPrimitive.Portal>
)
@@ -205,8 +210,11 @@ function DropdownMenuSubTrigger({
function DropdownMenuSubContent({
className,
style,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof DropdownMenuPrimitive.SubContent>) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<DropdownMenuPrimitive.Portal>
<DropdownMenuPrimitive.SubContent
@@ -219,6 +227,7 @@ function DropdownMenuSubContent({
// would otherwise capture clicks when submenu overlaps it.
style={{ ...style, WebkitAppRegion: 'no-drag' } as React.CSSProperties}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
/>
</DropdownMenuPrimitive.Portal>
)
@@ -4,6 +4,7 @@ import * as React from 'react'
import { HoverCard as HoverCardPrimitive } from 'radix-ui'
import { cn } from '@/lib/utils'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function HoverCard({ ...props }: React.ComponentProps<typeof HoverCardPrimitive.Root>) {
return <HoverCardPrimitive.Root data-slot="hover-card" {...props} />
@@ -17,8 +18,11 @@ function HoverCardContent({
className,
align = 'center',
sideOffset = 4,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof HoverCardPrimitive.Content>) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<HoverCardPrimitive.Portal data-slot="hover-card-portal">
<HoverCardPrimitive.Content
@@ -35,6 +39,7 @@ function HoverCardContent({
className
)}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
/>
</HoverCardPrimitive.Portal>
)
@@ -5,6 +5,7 @@ import { Popover as PopoverPrimitive } from 'radix-ui'
import { cn } from '@/lib/utils'
import { updatePopoverContentRef } from './popover-content-ref'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function Popover(props: React.ComponentProps<typeof PopoverPrimitive.Root>) {
return <PopoverPrimitive.Root data-slot="popover" {...props} />
@@ -25,6 +26,7 @@ function PopoverContent({
portalContainer,
style,
onWheel,
onEscapeKeyDown,
ref: forwardedRef,
...props
}: React.ComponentProps<typeof PopoverPrimitive.Content> & {
@@ -87,6 +89,8 @@ function PopoverContent({
[onWheel]
)
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<PopoverPrimitive.Portal container={portalContainer ?? undefined}>
<PopoverPrimitive.Content
@@ -112,6 +116,7 @@ function PopoverContent({
} as React.CSSProperties
}
onWheel={handleWheel}
onEscapeKeyDown={handleEscapeKeyDown}
{...props}
/>
</PopoverPrimitive.Portal>
@@ -5,6 +5,7 @@ import { CheckIcon, ChevronDownIcon, ChevronUpIcon } from 'lucide-react'
import { Select as SelectPrimitive } from 'radix-ui'
import { cn } from '@/lib/utils'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function Select({ ...props }: React.ComponentProps<typeof SelectPrimitive.Root>) {
return <SelectPrimitive.Root data-slot="select" {...props} />
@@ -50,10 +51,13 @@ function SelectContent({
position = 'item-aligned',
align = 'center',
portalContainer,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof SelectPrimitive.Content> & {
portalContainer?: HTMLElement | null
}) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<SelectPrimitive.Portal container={portalContainer ?? undefined}>
<SelectPrimitive.Content
@@ -71,6 +75,7 @@ function SelectContent({
position={position}
align={align}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
>
<SelectScrollUpButton />
<SelectPrimitive.Viewport
+5
View File
@@ -7,6 +7,7 @@ import { cva, type VariantProps } from 'class-variance-authority'
import { cn } from '@/lib/utils'
import { translate } from '@/i18n/i18n'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function Sheet({ ...props }: React.ComponentProps<typeof SheetPrimitive.Root>) {
return <SheetPrimitive.Root data-slot="sheet" {...props} />
@@ -79,6 +80,7 @@ function SheetContent({
overlayClassName,
overlayStyle,
style,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof SheetPrimitive.Content> &
VariantProps<typeof sheetContentVariants> & {
@@ -86,6 +88,8 @@ function SheetContent({
overlayClassName?: string
overlayStyle?: React.CSSProperties
}) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<SheetPortal>
<SheetOverlay className={overlayClassName} style={overlayStyle} />
@@ -96,6 +100,7 @@ function SheetContent({
// document root and its header overlaps the titlebar drag strip.
style={{ ...style, WebkitAppRegion: 'no-drag' } as React.CSSProperties}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
>
{children}
{showCloseButton && (
@@ -2,6 +2,7 @@ import * as React from 'react'
import { Tooltip as TooltipPrimitive } from 'radix-ui'
import { cn } from '@/lib/utils'
import { useImeAwareEscapeKeyDown } from './use-ime-aware-escape-key-down'
function TooltipProvider({
delayDuration = 0,
@@ -32,8 +33,11 @@ function TooltipContent({
className,
sideOffset = 0,
children,
onEscapeKeyDown,
...props
}: React.ComponentProps<typeof TooltipPrimitive.Content>) {
const handleEscapeKeyDown = useImeAwareEscapeKeyDown(onEscapeKeyDown)
return (
<TooltipPrimitive.Portal>
<TooltipPrimitive.Content
@@ -46,6 +50,7 @@ function TooltipContent({
className
)}
{...props}
onEscapeKeyDown={handleEscapeKeyDown}
>
{children}
<TooltipPrimitive.Arrow className="size-2.5 translate-y-[calc(-50%_-_2px)] rotate-45 rounded-[2px] bg-foreground fill-foreground" />
@@ -0,0 +1,21 @@
import * as React from 'react'
import { isImeCompositionKeyDown } from '@/lib/ime-composition-keyboard-event'
type EscapeKeyDownHandler = (event: KeyboardEvent) => void
export function useImeAwareEscapeKeyDown(
onEscapeKeyDown?: EscapeKeyDownHandler
): EscapeKeyDownHandler {
return React.useCallback(
(event) => {
// Radix observes Escape during document capture, before element keydown handlers.
if (isImeCompositionKeyDown(event)) {
event.preventDefault()
return
}
onEscapeKeyDown?.(event)
},
[onEscapeKeyDown]
)
}
@@ -9,14 +9,28 @@ import { describe, expect, it } from 'vitest'
//
// Ordering is a property of the source, not of any one component's behaviour, so it is pinned
// here once rather than re-tested in every composer that happens to handle Escape.
// It enforces textual guard-first order, including equivalent `||` bails, so the heuristic has
// one auditable source shape instead of approximating short-circuit control flow.
const ROOTS = ['src/renderer/src', 'src/main', 'src/shared']
const GUARD = 'isImeCompositionKeyDown'
const ESCAPE = "=== 'Escape'"
const ESCAPE_COMPARISON = /[!=]==\s*['"]Escape['"]/
const MUST_GUARD_EVERY_ESCAPE_HANDLER = new Set([
'src/renderer/src/components/NewWorkspaceComposerModal.tsx',
'src/renderer/src/components/browser-pane/BrowserAddressBar.tsx',
'src/renderer/src/components/editor/EditorPanelHeaderPath.tsx',
'src/renderer/src/components/editor/RichMarkdownLinkBubble.tsx',
'src/renderer/src/components/github/GitHubMarkdownComposer.tsx',
'src/renderer/src/components/github-project/ProjectCell.tsx',
'src/renderer/src/components/mobile/use-mobile-page-escape.ts'
])
const MUST_SCAN = [
...MUST_GUARD_EVERY_ESCAPE_HANDLER,
'src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts',
'src/renderer/src/components/new-workspace/SmartWorkspaceNameField.tsx',
'src/renderer/src/components/settings/ShortcutRecorderButton.tsx'
'src/renderer/src/components/settings/ShortcutRecorderButton.tsx',
'src/renderer/src/components/sidebar/WorkspaceKanbanSearchField.tsx',
'src/renderer/src/lib/task-page-window-shortcut-policy.ts'
]
function sourceFiles(dir: string): string[] {
@@ -65,10 +79,10 @@ function bodyBrace(source: string, from: number): number {
return -1
}
// Returns the body of every keydown handler in `source`, brace-matched from its opening `{`.
function keydownHandlerBodies(source: string): string[] {
// Returns every function body in `source`, including inline JSX keydown handlers.
function functionBodies(source: string): string[] {
const bodies: string[] = []
const starts = /onKeyDown(?:Capture)?=\{|(?:function|const)\s+\w*[Kk]ey[Dd]own\w*\s*[=(]/g
const starts = /(?:on|handle)KeyDown(?:Capture)?\s*[:=]|(?:function|const)\s+\w+\s*[=(]/g
for (let match = starts.exec(source); match !== null; match = starts.exec(source)) {
const open = bodyBrace(source, match.index + match[0].length)
if (open === -1) {
@@ -91,25 +105,30 @@ function keydownHandlerBodies(source: string): string[] {
}
describe('composition guards sit above the Escape branch', () => {
const offenders: string[] = []
const offenders = new Set<string>()
for (const root of ROOTS) {
for (const file of sourceFiles(root)) {
const source = readFileSync(file, 'utf8')
if (!source.includes(GUARD) || !source.includes(ESCAPE)) {
if (!source.includes(GUARD) || !ESCAPE_COMPARISON.test(source)) {
continue
}
for (const body of keydownHandlerBodies(source)) {
for (const body of functionBodies(source)) {
const guardAt = body.indexOf(GUARD)
const escapeAt = body.indexOf(ESCAPE)
if (guardAt !== -1 && escapeAt !== -1 && escapeAt < guardAt) {
offenders.push(file)
const escapeAt = body.search(ESCAPE_COMPARISON)
const relativeFile = file.split(sep).join('/')
const requiresGuard = MUST_GUARD_EVERY_ESCAPE_HANDLER.has(relativeFile)
if (
escapeAt !== -1 &&
((requiresGuard && guardAt === -1) || (guardAt !== -1 && escapeAt < guardAt))
) {
offenders.add(file)
}
}
}
}
it('finds no keydown handler that dispatches Escape before consulting the guard', () => {
expect(offenders).toEqual([])
expect([...offenders]).toEqual([])
})
// A scanner that parses nothing reports zero offenders forever, so the coverage it claims is
@@ -120,11 +139,10 @@ describe('composition guards sit above the Escape branch', () => {
const scanned = ROOTS.flatMap(sourceFiles)
.filter((file) => {
const source = readFileSync(file, 'utf8')
return source.includes(GUARD) && keydownHandlerBodies(source).some((b) => b.includes(GUARD))
return source.includes(GUARD) && functionBodies(source).some((body) => body.includes(GUARD))
})
.map((file) => file.split(sep).join('/'))
expect(scanned).toEqual(expect.arrayContaining(MUST_SCAN))
expect(scanned.length).toBeGreaterThan(8)
})
})