fix(ui): preserve IME composition in comment inputs (#3228)
Thanks for extending the existing IME handling to comment inputs and documenting the browser-event checks. We will also protect the annotation Escape handlers from cancelling composition.
This commit is contained in:
@@ -0,0 +1,24 @@
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const inputSource = readFileSync(join(__dirname, 'InlineCommentInput.tsx'), 'utf-8');
|
||||
|
||||
describe('InlineCommentInput IME handling', () => {
|
||||
test('ignores composition keydown events before handling save shortcuts', () => {
|
||||
expect(inputSource).toContain("import { isIMECompositionEvent } from '@/lib/ime';");
|
||||
|
||||
const handlerStart = inputSource.indexOf('const handleKeyDown');
|
||||
const handlerEnd = inputSource.indexOf('const handleSaveClick', handlerStart);
|
||||
expect(handlerStart).toBeGreaterThan(-1);
|
||||
expect(handlerEnd).toBeGreaterThan(handlerStart);
|
||||
|
||||
const handler = inputSource.slice(handlerStart, handlerEnd);
|
||||
const imeGuard = handler.indexOf('if (isIMECompositionEvent(e)) return;');
|
||||
const saveShortcut = handler.indexOf("e.key === 'Enter'");
|
||||
expect(imeGuard).toBeGreaterThan(-1);
|
||||
expect(saveShortcut).toBeGreaterThan(imeGuard);
|
||||
});
|
||||
});
|
||||
@@ -3,6 +3,7 @@ import { cn } from '@/lib/utils';
|
||||
import { Icon } from '@/components/icon/Icon';
|
||||
import { useDeviceInfo } from '@/lib/device';
|
||||
import { useI18n } from '@/lib/i18n';
|
||||
import { isIMECompositionEvent } from '@/lib/ime';
|
||||
import { formatShortcutForDisplay } from '@/lib/shortcuts';
|
||||
|
||||
export interface InlineCommentInputProps {
|
||||
@@ -121,6 +122,8 @@ export function InlineCommentInput({
|
||||
};
|
||||
|
||||
const handleKeyDown = (e: React.KeyboardEvent) => {
|
||||
if (isIMECompositionEvent(e)) return;
|
||||
|
||||
// As the placeholder promises: Cmd/Ctrl+Enter attaches, plain Enter
|
||||
// breaks the line, Escape cancels.
|
||||
if ((e.metaKey || e.ctrlKey) && e.key === 'Enter') {
|
||||
|
||||
@@ -67,6 +67,19 @@ describe('annotation overlay script', () => {
|
||||
expect(script).toContain('event.stopPropagation();');
|
||||
});
|
||||
|
||||
test('does not attach a comment while IME composition is active', () => {
|
||||
const handlerStart = script.indexOf('var onCommentKeyDown = function (event) {');
|
||||
const handlerEnd = script.indexOf('};', handlerStart);
|
||||
expect(handlerStart).toBeGreaterThan(-1);
|
||||
expect(handlerEnd).toBeGreaterThan(handlerStart);
|
||||
|
||||
const handler = script.slice(handlerStart, handlerEnd);
|
||||
const imeGuard = handler.indexOf('event.isComposing || event.keyCode === 229');
|
||||
const attachCall = handler.indexOf('attach();');
|
||||
expect(imeGuard).toBeGreaterThan(-1);
|
||||
expect(attachCall).toBeGreaterThan(imeGuard);
|
||||
});
|
||||
|
||||
test('escapes a label that would otherwise close the script', () => {
|
||||
const hostile = buildAnnotationOverlayScript(theme, {
|
||||
...labels,
|
||||
|
||||
@@ -545,6 +545,8 @@ export const buildAnnotationOverlayScript = (
|
||||
var onCommentKeyDown = function (event) {
|
||||
// Do not let the annotated page treat typed letters as its own shortcuts.
|
||||
event.stopPropagation();
|
||||
// WebKit can report the composition-confirming Enter as keyCode 229.
|
||||
if (event.isComposing || event.keyCode === 229) return;
|
||||
if (event.key === 'Enter' && !event.shiftKey) {
|
||||
event.preventDefault();
|
||||
attach();
|
||||
|
||||
Reference in New Issue
Block a user