fix(ui): guard the remaining comment inputs against IME composition
The IME fix covered the diff/file comment input and the browser annotation overlay, but two other places where a user writes a comment still acted on the Enter that confirms an IME candidate: the chat quote comment in the text selection menu, and the in-place comment editor on a composer context chip. Confirming a candidate there attached or committed the half-typed reading and closed the input. Escape, which abandons a candidate, had the same problem. Both handlers now return early on a composing keystroke, and a single test asserts the guard across every comment input so the next one added does not quietly skip it.
This commit is contained in:
@@ -14,6 +14,7 @@ import React from 'react';
|
|||||||
import { Icon } from '@/components/icon/Icon';
|
import { Icon } from '@/components/icon/Icon';
|
||||||
import type { IconName } from '@/components/icon/icons';
|
import type { IconName } from '@/components/icon/icons';
|
||||||
import { useI18n } from '@/lib/i18n';
|
import { useI18n } from '@/lib/i18n';
|
||||||
|
import { isIMECompositionEvent } from '@/lib/ime';
|
||||||
import { getRuntimeKey } from '@/lib/runtime-switch';
|
import { getRuntimeKey } from '@/lib/runtime-switch';
|
||||||
import {
|
import {
|
||||||
EMPTY_INLINE_COMMENT_DRAFTS,
|
EMPTY_INLINE_COMMENT_DRAFTS,
|
||||||
@@ -165,6 +166,10 @@ const DraftPreviewEntry: React.FC<{
|
|||||||
onChange={(event) => setEditText(event.target.value)}
|
onChange={(event) => setEditText(event.target.value)}
|
||||||
onBlur={commitEdit}
|
onBlur={commitEdit}
|
||||||
onKeyDown={(event) => {
|
onKeyDown={(event) => {
|
||||||
|
// An IME candidate is confirmed with Enter and
|
||||||
|
// abandoned with Escape; neither keystroke should
|
||||||
|
// commit or revert the edit.
|
||||||
|
if (isIMECompositionEvent(event)) return;
|
||||||
if (event.key === 'Enter' && !event.shiftKey) {
|
if (event.key === 'Enter' && !event.shiftKey) {
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
commitEdit();
|
commitEdit();
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ import { resolveProjectForSessionDirectory } from '@/lib/projectResolution';
|
|||||||
import { useEffectiveDirectory } from '@/hooks/useEffectiveDirectory';
|
import { useEffectiveDirectory } from '@/hooks/useEffectiveDirectory';
|
||||||
import { isVSCodeRuntime } from '@/lib/desktop';
|
import { isVSCodeRuntime } from '@/lib/desktop';
|
||||||
import { useI18n } from '@/lib/i18n';
|
import { useI18n } from '@/lib/i18n';
|
||||||
|
import { isIMECompositionEvent } from '@/lib/ime';
|
||||||
import { rangeToMarkdown, trimSelectionValue, wrapMarkdownSelectionForChat } from './selectionMarkdown';
|
import { rangeToMarkdown, trimSelectionValue, wrapMarkdownSelectionForChat } from './selectionMarkdown';
|
||||||
import { focusChatInput } from '@/components/chat/composer/editor/dom';
|
import { focusChatInput } from '@/components/chat/composer/editor/dom';
|
||||||
import { registerActiveSelectionToolbar } from '@/lib/addSelectionToChat';
|
import { registerActiveSelectionToolbar } from '@/lib/addSelectionToChat';
|
||||||
@@ -590,6 +591,9 @@ export const TextSelectionMenu: React.FC<TextSelectionMenuProps> = ({ containerR
|
|||||||
resizeCommentInput();
|
resizeCommentInput();
|
||||||
}}
|
}}
|
||||||
onKeyDown={(event) => {
|
onKeyDown={(event) => {
|
||||||
|
// An IME candidate is confirmed with Enter and abandoned with
|
||||||
|
// Escape; neither keystroke belongs to the comment yet.
|
||||||
|
if (isIMECompositionEvent(event)) return;
|
||||||
// Desktop: Enter attaches, Shift+Enter breaks the line. Mobile
|
// Desktop: Enter attaches, Shift+Enter breaks the line. Mobile
|
||||||
// keyboards use Enter for line breaks; attaching is the button's job.
|
// keyboards use Enter for line breaks; attaching is the button's job.
|
||||||
if (event.key === 'Enter' && !event.shiftKey && !isMobile) {
|
if (event.key === 'Enter' && !event.shiftKey && !isMobile) {
|
||||||
|
|||||||
@@ -0,0 +1,64 @@
|
|||||||
|
import { describe, expect, test } from 'bun:test';
|
||||||
|
import { readFileSync } from 'node:fs';
|
||||||
|
import { dirname, join } from 'node:path';
|
||||||
|
import { fileURLToPath } from 'node:url';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Every place a user writes a comment submits on a bare Enter, and Enter is
|
||||||
|
* also how an IME confirms a candidate. Without a composition guard the first
|
||||||
|
* confirmation posts the half-typed reading and closes the input, so the guard
|
||||||
|
* is a contract across all of these surfaces rather than a per-component
|
||||||
|
* detail. `keyCode === 229` is part of it: WebKit reports the confirming Enter
|
||||||
|
* that way after `compositionend`.
|
||||||
|
*/
|
||||||
|
const srcDir = join(dirname(fileURLToPath(import.meta.url)), '..');
|
||||||
|
|
||||||
|
const COMMENT_INPUTS: Array<{ name: string; file: string; handler: string; guard: RegExp }> = [
|
||||||
|
{
|
||||||
|
name: 'diff and file comments',
|
||||||
|
file: 'components/comments/InlineCommentInput.tsx',
|
||||||
|
handler: 'const handleKeyDown',
|
||||||
|
guard: /isIMECompositionEvent\(e\)\) return;/,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'chat quote comments',
|
||||||
|
file: 'components/chat/message/TextSelectionMenu.tsx',
|
||||||
|
handler: 'ref={commentInputRef}',
|
||||||
|
guard: /isIMECompositionEvent\(event\)\) return;/,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'composer context chip editor',
|
||||||
|
file: 'components/chat/composer/ui/ComposerContextChips.tsx',
|
||||||
|
handler: 'ref={editRef}',
|
||||||
|
guard: /isIMECompositionEvent\(event\)\) return;/,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'browser annotation input',
|
||||||
|
file: 'lib/browser/annotationOverlay.ts',
|
||||||
|
handler: 'var onCommentKeyDown = function (event) {',
|
||||||
|
guard: /event\.isComposing \|\| event\.keyCode === 229\) return;/,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: 'browser annotation escape',
|
||||||
|
file: 'lib/browser/annotationOverlay.ts',
|
||||||
|
handler: 'var onKeyDown = function (event) {',
|
||||||
|
guard: /event\.isComposing \|\| event\.keyCode === 229\) return;/,
|
||||||
|
},
|
||||||
|
];
|
||||||
|
|
||||||
|
describe('comment inputs ignore IME composition keystrokes', () => {
|
||||||
|
for (const input of COMMENT_INPUTS) {
|
||||||
|
test(input.name, () => {
|
||||||
|
const source = readFileSync(join(srcDir, input.file), 'utf-8');
|
||||||
|
const start = source.indexOf(input.handler);
|
||||||
|
expect(start).toBeGreaterThan(-1);
|
||||||
|
|
||||||
|
const handler = source.slice(start, start + 900);
|
||||||
|
const guardIndex = handler.search(input.guard);
|
||||||
|
const keyIndex = handler.search(/(event|e)\.key === '(Enter|Escape)'/);
|
||||||
|
|
||||||
|
expect(guardIndex).toBeGreaterThan(-1);
|
||||||
|
expect(keyIndex).toBeGreaterThan(guardIndex);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user