Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/quiet-inputs-compose.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@tailor-platform/app-shell": patch
---

Prevent IME confirmation Enter from applying DataTable filters or closing their value editors mid-input.
Use consistent IME detection, including the keyCode 229 fallback, in filters, cell editors, the command palette, and AI chat.
22 changes: 12 additions & 10 deletions packages/core/src/components/ai-chat/ai-chat.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useState } from "react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { cleanup, render, screen } from "@testing-library/react";
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";

import { AIChat } from "./ai-chat";
Expand Down Expand Up @@ -253,19 +253,21 @@ describe("AIChat", () => {
expect(onSubmit).not.toHaveBeenCalled();
});

it("does not submit on Enter during IME composition", async () => {
it.each([
{ isComposing: true, keyCode: 13 },
{ isComposing: false, keyCode: 229 },
])("ignores IME Enter (%j) but submits on normal Enter", async (ime) => {
const user = userEvent.setup();
const { onSubmit } = renderComposer();
await user.type(textbox(), "変換");
textbox().dispatchEvent(
new KeyboardEvent("keydown", {
key: "Enter",
bubbles: true,
cancelable: true,
isComposing: true,
}),
);

fireEvent.keyDown(textbox(), { key: "Enter", ...ime });
expect(onSubmit).not.toHaveBeenCalled();
expect(textbox().value).toBe("変換");

await user.keyboard("{Enter}");
expect(onSubmit).toHaveBeenCalledExactlyOnceWith("変換");
expect(textbox().value).toBe("");
});

it("disables submit while the draft is empty or whitespace-only", async () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/components/ai-chat/composer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { Button } from "@/components/button";
import { Textarea } from "@/components/textarea";
import { useT } from "@/i18n-labels";
import { cn } from "@/lib/utils";
import { isComposing } from "@/lib/is-composing";
import { useAIChatContext } from "./ai-chat-context";

type AIChatComposerProps = {
Expand Down Expand Up @@ -121,7 +122,7 @@ function Composer({
submitOnEnter &&
event.key === "Enter" &&
!event.shiftKey &&
!event.nativeEvent.isComposing
!isComposing(event.nativeEvent)
) {
event.preventDefault();
event.currentTarget.form?.requestSubmit();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,23 +161,22 @@ describe("CommandPaletteContent Integration", () => {
});
});

it("ignores Enter during IME composition", async () => {
it.each([
{ isComposing: true, keyCode: 13 },
{ isComposing: false, keyCode: 229 },
])("ignores IME Enter (%j) but navigates on normal Enter", async (ime) => {
renderCommandPaletteContent();

fireEvent.keyDown(document, { key: "k", metaKey: true });
const dialog = await screen.findByRole("dialog");

// Simulate IME composition
const enterEvent = new KeyboardEvent("keydown", {
key: "Enter",
bubbles: true,
isComposing: true,
});

dialog.dispatchEvent(enterEvent);

// Dialog should remain open
fireEvent.keyDown(dialog, { key: "Enter", ...ime });
expect(screen.getByPlaceholderText("Search pages...")).toBeDefined();

fireEvent.keyDown(dialog, { key: "Enter" });
await vi.waitFor(() => {
expect(screen.queryByPlaceholderText("Search pages...")).toBeNull();
});
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Input } from "@/components/input";
import { Spinner } from "@/components/spinner";
import { useT } from "@/i18n-labels";
import { cn } from "@/lib/utils";
import { isComposing } from "@/lib/is-composing";
import { filterRoutes, NavigatableRoute } from "@/routing/path";
import { useNavItems, NavItem, NavItemResource } from "../../routing/navigation";
import {
Expand Down Expand Up @@ -460,8 +461,7 @@ export function useCommandPalette({

const handleKeyDown = useCallback(
(e: React.KeyboardEvent) => {
// Ignore during IME composition
if (e.nativeEvent.isComposing) return;
if (isComposing(e.nativeEvent)) return;

switch (e.key) {
case "ArrowDown":
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/components/data-table/editable-cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { Popover } from "@base-ui/react/popover";
import type { CalendarDate } from "@internationalized/date";
import { CalendarDays, ChevronDown } from "lucide-react";
import { cn } from "@/lib/utils";
import { isComposing } from "@/lib/is-composing";
import { useTimeZone } from "@/contexts/appshell-context";
import { BadgeList } from "@/components/badge-list";
import { Button } from "@/components/button";
Expand Down Expand Up @@ -399,7 +400,7 @@ function TypingEditCell<TRow extends Record<string, unknown>>({

const handleKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
// While an IME is composing, Enter confirms the conversion, not the cell.
if (event.nativeEvent.isComposing || composingRef.current || event.keyCode === 229) return;
if (isComposing(event.nativeEvent, composingRef.current)) return;
const from = { rowKey, colKey };
switch (event.key) {
case "Enter":
Expand Down Expand Up @@ -670,7 +671,7 @@ function ChoiceEditCell<TRow extends Record<string, unknown>>({
const handleKeyDown: NonNullable<ComponentProps<typeof ComboboxParts.Input>["onKeyDown"]> = (
event,
) => {
if (event.nativeEvent.isComposing || composingRef.current || event.keyCode === 229) {
if (isComposing(event.nativeEvent, composingRef.current)) {
event.preventBaseUIHandler();
return;
}
Expand Down
84 changes: 84 additions & 0 deletions packages/core/src/components/data-table/toolbar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,90 @@ const stringInvalidOperatorColumn: Column<TestRow> = {
},
};

describe.each([
{ isComposing: true, keyCode: 13 },
{ isComposing: false, keyCode: 229 },
])("filter IME handling ($isComposing, $keyCode)", (compositionEvent) => {
describe.each(["panel", "chip"] as const)("%s", (surface) => {
it.each([
{ column: stringColumn, initial: "Alice", draft: "株式会社", final: "株式会社東京" },
{ column: uuidColumn, initial: "uuid-1", draft: "uuid-2", final: "uuid-3" },
{ column: numberColumn, initial: 1, draft: "2", final: 3 },
{ column: timeColumn, initial: "09:30", draft: "10:30", final: "11:30" },
])(
"does not commit $column.id until Enter outside composition",
async ({ column, initial, draft, final }) => {
const user = userEvent.setup();
const field = column.id!;
const operator = column === stringColumn ? "contains" : "eq";
const control = makeControl({ filters: [{ field, operator, value: initial }] });
render(<TestFilters control={control} columns={[column]} />, { wrapper });

if (surface === "panel") {
await user.click(screen.getByRole("button", { name: "Add filter" }));
} else {
await openValueEditor(user);
}

const input = await screen.findByDisplayValue(String(initial));
await user.click(input);
fireEvent.change(input, { target: { value: draft } });
fireEvent.keyDown(input, { key: "Enter", ...compositionEvent });

expect(control.addFilter).not.toHaveBeenCalled();
expect(control.removeFilter).not.toHaveBeenCalled();
expect(document.activeElement).toBe(input);
expect(screen.getByDisplayValue(draft)).toBe(input);

fireEvent.change(input, { target: { value: String(final) } });
fireEvent.keyDown(input, { key: "Enter", ...compositionEvent });
expect(control.addFilter).not.toHaveBeenCalled();
expect(control.removeFilter).not.toHaveBeenCalled();

fireEvent.keyDown(input, { key: "Enter", isComposing: false, keyCode: 13 });
expect(control.addFilter).toHaveBeenCalledTimes(1);
expect(vi.mocked(control.addFilter).mock.calls[0].slice(0, 3)).toEqual([
field,
operator,
final,
]);
if (surface === "chip") {
await vi.waitFor(() => expect(screen.queryByDisplayValue(String(final))).toBeNull());
} else {
expect(screen.getByDisplayValue(String(final))).toBe(input);
}
},
);

it.each(["Min", "Max"])("does not commit a range from %s during composition", async (bound) => {
const user = userEvent.setup();
const control = makeControl({
filters: [{ field: "count", operator: "between", value: { min: 1, max: 10 } }],
});
render(<TestFilters control={control} columns={[numberColumn]} />, { wrapper });
if (surface === "panel") {
await user.click(screen.getByRole("button", { name: "Add filter" }));
} else {
await openValueEditor(user);
}

const input = await screen.findByRole("spinbutton", { name: bound });
fireEvent.change(input, { target: { value: bound === "Min" ? "2" : "20" } });
fireEvent.keyDown(input, { key: "Enter", ...compositionEvent });
expect(control.addFilter).not.toHaveBeenCalled();
expect(control.removeFilter).not.toHaveBeenCalled();
expect(screen.getByRole("spinbutton", { name: bound })).toBe(input);

fireEvent.keyDown(input, { key: "Enter", isComposing: false, keyCode: 13 });
expect(control.addFilter).toHaveBeenCalledTimes(1);
expect(control.addFilter).toHaveBeenCalledWith("count", "between", {
min: bound === "Min" ? 2 : 1,
max: bound === "Max" ? 20 : 10,
});
});
});
});

// ---------------------------------------------------------------------------
// DataTable.Filters — rendering
// ---------------------------------------------------------------------------
Expand Down
17 changes: 9 additions & 8 deletions packages/core/src/components/data-table/toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useCallback, useMemo, useState, type ReactNode } from "react";
import { Popover } from "@base-ui/react/popover";
import { ChevronDown, Filter as FilterIcon, X, Check, Search } from "lucide-react";
import { cn } from "@/lib/utils";
import { isComposing } from "@/lib/is-composing";
import { useCollectionControlOptional } from "@/contexts/collection-control-context";
import { Button } from "@/components/button";
import { Input } from "@/components/input";
Expand Down Expand Up @@ -931,7 +932,7 @@ function PanelValueEditor({
value={text}
onChange={(e) => setText(e.target.value)}
onKeyDown={(e) => {
if (e.key === "Enter") apply();
if (e.key === "Enter" && !isComposing(e.nativeEvent)) apply();
}}
className="astw:h-8 astw:text-sm"
/>
Expand All @@ -947,7 +948,7 @@ function PanelValueEditor({
placeholder={t("filterValuePlaceholder", { field: label })}
onChange={(e) => setText(e.target.value)}
onKeyDown={(e) => {
if (e.key === "Enter") apply();
if (e.key === "Enter" && !isComposing(e.nativeEvent)) apply();
}}
className="astw:h-8 astw:text-sm"
/>
Expand Down Expand Up @@ -1031,7 +1032,7 @@ function BetweenInputGroup({
onChange={(e) => onChangeMin(e.target.value)}
onKeyDown={(e) => {
e.stopPropagation();
if (e.key === "Enter") onSubmit();
if (e.key === "Enter" && !isComposing(e.nativeEvent)) onSubmit();
}}
className={inputCell}
/>
Expand All @@ -1046,7 +1047,7 @@ function BetweenInputGroup({
onChange={(e) => onChangeMax(e.target.value)}
onKeyDown={(e) => {
e.stopPropagation();
if (e.key === "Enter") onSubmit();
if (e.key === "Enter" && !isComposing(e.nativeEvent)) onSubmit();
}}
className={inputCell}
/>
Expand Down Expand Up @@ -1732,7 +1733,7 @@ function StringFilterEditor({
onChange={(e) => setLocalValue(e.target.value)}
onKeyDown={(e) => {
e.stopPropagation();
if (e.key === "Enter") handleCommit();
if (e.key === "Enter" && !isComposing(e.nativeEvent)) handleCommit();
}}
className="astw:h-8 astw:text-sm"
/>
Expand Down Expand Up @@ -1784,7 +1785,7 @@ function UuidFilterEditor({
onChange={(e) => setLocalValue(e.target.value)}
onKeyDown={(e) => {
e.stopPropagation();
if (e.key === "Enter") handleCommit();
if (e.key === "Enter" && !isComposing(e.nativeEvent)) handleCommit();
}}
className="astw:h-8 astw:text-sm"
/>
Expand Down Expand Up @@ -1936,7 +1937,7 @@ function NumericFilterEditor({
onChange={(e) => setLocalValue(e.target.value)}
onKeyDown={(e) => {
e.stopPropagation();
if (e.key === "Enter") handleCommit();
if (e.key === "Enter" && !isComposing(e.nativeEvent)) handleCommit();
}}
className="astw:h-8 astw:text-sm"
/>
Expand Down Expand Up @@ -2120,7 +2121,7 @@ function TemporalFilterEditor({
}}
onKeyDown={(e) => {
e.stopPropagation();
if (e.key === "Enter") {
if (e.key === "Enter" && !isComposing(e.nativeEvent)) {
handleCommit();
}
}}
Expand Down
17 changes: 17 additions & 0 deletions packages/core/src/lib/is-composing.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { describe, expect, it } from "vitest";
import { isComposing } from "./is-composing";

describe("isComposing", () => {
it.each([
{ isComposing: false, keyCode: 13, composing: false, expected: false },
{ isComposing: true, keyCode: 13, composing: false, expected: true },
{ isComposing: false, keyCode: 229, composing: false, expected: true },
{ isComposing: false, keyCode: 13, composing: true, expected: true },
])("detects IME events: %o", ({ composing, expected, ...event }) => {
expect(isComposing(new KeyboardEvent("keydown", event), composing)).toBe(expected);
});

it("defaults to the native event when no composition state is supplied", () => {
expect(isComposing(new KeyboardEvent("keydown", { key: "Enter" }))).toBe(false);
});
});
4 changes: 4 additions & 0 deletions packages/core/src/lib/is-composing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export function isComposing(event: KeyboardEvent, composing = false): boolean {
// Some browsers end composition before keydown; keyCode 229 still identifies the IME event.
return event.isComposing || composing || event.keyCode === 229;
}
Loading