From aec8ec5f7935be29d37bb94150153d3e9d6388a4 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Thu, 22 Oct 2026 13:36:15 +0900 Subject: [PATCH 1/2] feat(data-table): add declarative cell-editing errors --- .changeset/quiet-cell-errors.md | 6 + docs-manifest.json | 6 +- .../components/data-table.docs.outline.md | 93 ++- docs/components/data-table.md | 93 ++- .../data-table-lab/editing-errors-demo.tsx | 149 ++++ .../pages/showcase/data-table-lab/page.tsx | 13 + .../data-table/data-table-editing.test.tsx | 6 +- .../data-table/data-table-errors.test.tsx | 733 ++++++++++++++++++ .../components/data-table/editable-cell.tsx | 470 ++++++++--- .../core/src/components/data-table/i18n.ts | 10 + .../core/src/components/data-table/types.ts | 26 +- .../data-table/use-cell-edit-session.test.tsx | 350 +++++++++ .../data-table/use-cell-edit-session.ts | 166 ++++ 13 files changed, 1948 insertions(+), 173 deletions(-) create mode 100644 .changeset/quiet-cell-errors.md create mode 100644 examples/vite-app/src/pages/showcase/data-table-lab/editing-errors-demo.tsx create mode 100644 packages/core/src/components/data-table/data-table-errors.test.tsx create mode 100644 packages/core/src/components/data-table/use-cell-edit-session.test.tsx create mode 100644 packages/core/src/components/data-table/use-cell-edit-session.ts diff --git a/.changeset/quiet-cell-errors.md b/.changeset/quiet-cell-errors.md new file mode 100644 index 000000000..70660ba69 --- /dev/null +++ b/.changeset/quiet-cell-errors.md @@ -0,0 +1,6 @@ +--- +"@tailor-platform/app-shell": minor +--- + +Show cell-local error popovers by default when DataTable editing callbacks throw or return a rejected Promise. Keep pending and failed drafts editable, wait for a successful `onValueChange` check before confirmation, and clear drafts only after confirmation succeeds; Esc or Revert edit discards the draft and error together without trapping cell navigation. +Customize the popover body with `edit.error.render({ row, error })`, or set `edit.error: false` to retain eager edit completion without callback interception. Data adoption, persistence, and rollback remain consumer-owned. diff --git a/docs-manifest.json b/docs-manifest.json index d77b51ac4..a6496cd99 100644 --- a/docs-manifest.json +++ b/docs-manifest.json @@ -454,9 +454,9 @@ ], "hashes": { "typeSurface": "3ca8fce7d2efca2d", - "outline": "0b7a57e7d2fc04a5", + "outline": "34a240cc8b38c38e", "snapshot": null, - "outputMd": "1073852822f9e535", + "outputMd": "23265474c7780224", "examples": "b3468077b9455521" } }, @@ -1751,7 +1751,7 @@ "packages/core/skills/app-shell-patterns/references/components/combobox.md": "dfb775c7c4307409", "packages/core/skills/app-shell-patterns/references/components/command-palette.md": "7e5bc675a593791a", "packages/core/skills/app-shell-patterns/references/components/csv-importer.md": "53d3c795ca21b9dc", - "packages/core/skills/app-shell-patterns/references/components/data-table.md": "1073852822f9e535", + "packages/core/skills/app-shell-patterns/references/components/data-table.md": "23265474c7780224", "packages/core/skills/app-shell-patterns/references/components/date-picker.md": "bc69f14829004f9d", "packages/core/skills/app-shell-patterns/references/components/default-header.md": "375e55f4b12fa2be", "packages/core/skills/app-shell-patterns/references/components/default-sidebar.md": "3422388a0cc78571", diff --git a/docs-src/components/data-table.docs.outline.md b/docs-src/components/data-table.docs.outline.md index 22bd9abce..90e8b227d 100644 --- a/docs-src/components/data-table.docs.outline.md +++ b/docs-src/components/data-table.docs.outline.md @@ -385,7 +385,7 @@ The trigger is a native ` + + ), + }, + }, + }), + ]; + const table = useDataTable({ columns, data: { rows } }); + + return ( +
+ + + Click an error icon or press Alt+ArrowDown in its editor to inspect the popover without + confirming or discarding the draft. Pending and failed drafts survive Tab or blur without + trapping focus. Only successful confirmation clears the draft; only the consumer adopts + successful values. Closing error details keeps the draft; Esc in the editor or Revert edit + cancels the draft and its error together. + + + + +
+ + Saves in flight: {savingCount} + + }> + {feedback} + +
+ + +
+ ); +} diff --git a/examples/vite-app/src/pages/showcase/data-table-lab/page.tsx b/examples/vite-app/src/pages/showcase/data-table-lab/page.tsx index 728207fcc..c2c3aae2a 100644 --- a/examples/vite-app/src/pages/showcase/data-table-lab/page.tsx +++ b/examples/vite-app/src/pages/showcase/data-table-lab/page.tsx @@ -12,6 +12,7 @@ import { import { FlaskConical } from "lucide-react"; import { useMemo } from "react"; import { InlineEditingDemo } from "./inline-editing-demo"; +import { EditingErrorsDemo } from "./editing-errors-demo"; // ─── Dummy data ────────────────────────────────────────────────────────────── // 🧪 Dummy Data: Replace with a real GraphQL-backed source later. @@ -574,6 +575,18 @@ const DataTableLabPage = () => { +
+ Callback failures use a standard cell-error popover by default. Only the Custom code + column supplies error.render; the other columns need no error config. + + } + > + +
+
{ expect(input.closest("td")?.textContent).toContain("ABC"); }); - it("does not keep optimistic cell state or wait for a returned promise", async () => { + it("retains a pending draft without adopting it or blocking focus movement", async () => { let finish = noop; const onValueCommitted = vi.fn(() => new Promise((resolve) => (finish = resolve))); const user = renderTable( @@ -607,10 +607,12 @@ describe("DataTable inline editing", () => { const [first, second] = screen.getAllByRole("textbox", { name: "Received" }); await retype(user, first, "20{Enter}"); expect(onValueCommitted).toHaveBeenCalledTimes(1); - expect((first as HTMLInputElement).value).toBe("12"); + expect((first as HTMLInputElement).value).toBe("20"); expect(document.activeElement).toBe(first); await user.tab(); expect(document.activeElement).toBe(second); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + expect((first as HTMLInputElement).value).toBe("20"); await act(async () => finish()); expect((first as HTMLInputElement).value).toBe("12"); }); diff --git a/packages/core/src/components/data-table/data-table-errors.test.tsx b/packages/core/src/components/data-table/data-table-errors.test.tsx new file mode 100644 index 000000000..68ee83969 --- /dev/null +++ b/packages/core/src/components/data-table/data-table-errors.test.tsx @@ -0,0 +1,733 @@ +import { StrictMode } from "react"; +import { afterEach, describe, expect, expectTypeOf, it, vi } from "vitest"; +import { act, cleanup, fireEvent, render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { MemoryRouter } from "react-router"; +import type { TextCellEditOptions } from "@/index"; +import { createAppShellWrapper } from "../../../tests/test-utils"; +import { DataTable } from "./data-table"; +import { useDataTable } from "./use-data-table"; +import type { Column } from "./types"; + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +type Row = { id: string; value: string | null }; +const ROWS: Row[] = [{ id: "1", value: "Original" }]; +const noop = () => {}; + +function Harness({ edit, rows = ROWS }: { edit: TextCellEditOptions; rows?: Row[] }) { + const columns: Column[] = [{ id: "value", label: "Value", type: "text", edit }]; + const table = useDataTable({ columns, data: { rows } }); + return ( + <> + + + + + + ); +} + +function renderEditor( + edit: TextCellEditOptions, + { locale = "en", rows = ROWS }: { locale?: string; rows?: Row[] } = {}, +) { + const user = userEvent.setup(); + const ui = (nextRows: Row[]) => ( + + + + + + ); + const view = render(ui(rows), { wrapper: createAppShellWrapper(locale) }); + const inputs = screen.getAllByRole("textbox", { name: "Value" }) as HTMLInputElement[]; + return { + ...view, + user, + input: inputs[0], + inputs, + updateRows: (nextRows: Row[]) => view.rerender(ui(nextRows)), + }; +} + +function change(input: HTMLInputElement, value = "Changed") { + act(() => input.focus()); + fireEvent.change(input, { target: { value } }); +} + +function deferred() { + let resolve!: () => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +async function rejectRequest(request: ReturnType, reason: unknown) { + await act(async () => { + request.reject(reason); + await request.promise.catch(noop); + }); +} + +async function resolveRequest(request: ReturnType) { + await act(async () => { + request.resolve(); + await request.promise; + }); +} + +function renderColumn(column: Column, rows = ROWS) { + function TableHarness() { + const table = useDataTable({ columns: [column], data: { rows } }); + return ( + + + + ); + } + render( + + + , + { wrapper: createAppShellWrapper("en") }, + ); + return userEvent.setup(); +} + +const errorTrigger = () => screen.getByRole("button", { name: "Show error for Value" }); +const queryErrorTrigger = () => screen.queryByRole("button", { name: "Show error for Value" }); + +describe("DataTable callback error presentation", () => { + it.each(["change", "commit"] as const)( + "shows a synchronous %s failure in the default popover without opting in", + async (source) => { + const failure = new Error("Cannot accept this value"); + const fail = vi.fn(() => { + throw failure; + }); + const onValueCommitted = source === "commit" ? fail : vi.fn(); + const { input, user } = renderEditor({ + onValueChange: source === "change" ? fail : undefined, + onValueCommitted, + }); + change(input); + if (source === "commit") fireEvent.keyDown(input, { key: "Enter" }); + + expect(input.getAttribute("aria-invalid")).toBe("true"); + expect( + document.getElementById(input.getAttribute("aria-describedby")!)?.textContent, + ).toContain(failure.message); + expect(screen.queryByRole("dialog")).toBeNull(); + await user.click(errorTrigger()); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + expect(within(popup).getByText(failure.message)).toBeDefined(); + expect(fail).toHaveBeenCalledExactlyOnceWith(ROWS[0], "Changed"); + expect(input.value).toBe("Changed"); + expect(input.readOnly).toBe(false); + if (source === "change") { + fireEvent.keyDown(input, { key: "Enter" }); + expect(onValueCommitted).not.toHaveBeenCalled(); + } + }, + ); + + it.each(["change", "commit"] as const)( + "retains the draft while a returned %s promise is pending and after rejection", + async (source) => { + const request = deferred(); + const { input, user } = renderEditor({ + onValueChange: source === "change" ? () => request.promise : undefined, + onValueCommitted: source === "commit" ? () => request.promise : noop, + }); + change(input); + if (source === "commit") { + fireEvent.keyDown(input, { key: "Enter" }); + expect(input.closest("td")?.getAttribute("aria-busy")).toBe("true"); + } + expect(input.readOnly).toBe(false); + expect(input.value).toBe("Changed"); + expect(queryErrorTrigger()).toBeNull(); + await rejectRequest(request, new Error("Server rejected the value")); + expect(input.value).toBe("Changed"); + expect(input.readOnly).toBe(false); + expect(input.closest("td")?.getAttribute("aria-busy")).toBeNull(); + await user.click(errorTrigger()); + expect( + within(await screen.findByRole("dialog", { name: "Error in Value" })).getByText( + "Server rejected the value", + ), + ).toBeDefined(); + }, + ); + + it("supports custom error content, keyboard inspection, and interactive actions without committing the draft", async () => { + const failure = { code: "INVALID" }; + const onValueCommitted = vi.fn(); + const onAction = vi.fn(); + const renderError = vi.fn(({ row, error }: { row: Row; error: unknown }) => ( + <> +

{`${row.id}: ${(error as typeof failure).code}`}

+ + + )); + const { input, user } = renderEditor({ + onValueChange: () => { + throw failure; + }, + onValueCommitted, + error: { render: renderError }, + }); + change(input); + fireEvent.keyDown(input, { key: "ArrowDown", altKey: true }); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + expect(within(popup).getByText("1: INVALID")).toBeDefined(); + expect(renderError).toHaveBeenCalledWith({ row: ROWS[0], error: failure }); + const action = within(popup).getByRole("button", { name: "Explain this error" }); + await vi.waitFor(() => expect(document.activeElement).toBe(action)); + await user.keyboard("{Enter}"); + expect(onAction).toHaveBeenCalledTimes(1); + expect(onValueCommitted).not.toHaveBeenCalled(); + expect(input.value).toBe("Changed"); + + await user.keyboard("{Escape}"); + await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + expect(onValueCommitted).not.toHaveBeenCalled(); + expect(input.value).toBe("Changed"); + await user.click(input); + expect(input.value).toBe("Changed"); + expect(onValueCommitted).not.toHaveBeenCalled(); + await user.click(screen.getByRole("button", { name: "Outside" })); + expect(onValueCommitted).not.toHaveBeenCalled(); + expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + }); + + it.each([null, undefined, false, "not an Error"])( + "uses a generic message for thrown %j instead of treating it as no error", + async (failure) => { + const { input, user } = renderEditor({ + onValueChange: () => { + throw failure; + }, + onValueCommitted: noop, + }); + change(input); + await user.click(errorTrigger()); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + expect(within(popup).getByText("An error occurred")).toBeDefined(); + }, + ); + + it("localizes the generic error and accessible popup labels", async () => { + const { input, user } = renderEditor( + { + onValueChange: () => { + throw undefined; + }, + onValueCommitted: noop, + }, + { locale: "ja" }, + ); + change(input); + await user.click(screen.getByRole("button", { name: "Valueのエラーを表示" })); + expect( + within(await screen.findByRole("dialog", { name: "Valueのエラー" })).getByText( + "エラーが発生しました", + ), + ).toBeDefined(); + }); + + it("clears an error on a newer successful input and ignores older rejection results", async () => { + const old = deferred(); + const current = deferred(); + const { input } = renderEditor({ + onValueChange: (_row, draft) => { + if (draft === "Older") return old.promise; + if (draft === "Current") return current.promise; + return undefined; + }, + onValueCommitted: noop, + }); + change(input, "Older"); + fireEvent.change(input, { target: { value: "Current" } }); + await rejectRequest(current, new Error("Current failure")); + expect(input.getAttribute("aria-invalid")).toBe("true"); + await rejectRequest(old, new Error("Outdated failure")); + expect(document.getElementById(input.getAttribute("aria-describedby")!)?.textContent).toContain( + "Current failure", + ); + fireEvent.change(input, { target: { value: "Newer" } }); + expect(queryErrorTrigger()).toBeNull(); + expect(input.getAttribute("aria-invalid")).toBeNull(); + }); + + it("does not attach a rejected save to a newer unfinished draft", async () => { + const request = deferred(); + const { input } = renderEditor({ onValueCommitted: () => request.promise }); + change(input); + fireEvent.keyDown(input, { key: "Enter" }); + fireEvent.keyDown(input, { key: "Enter" }); + fireEvent.change(input, { target: { value: "New draft" } }); + await rejectRequest(request, new Error("Old save failure")); + expect(input.value).toBe("New draft"); + expect(queryErrorTrigger()).toBeNull(); + }); + + it.each(["resolve", "reject"] as const)( + "waits for the latest change check to %s before attempting confirmation", + async (outcome) => { + const check = deferred(); + const onValueCommitted = vi.fn(); + const { input } = renderEditor({ + onValueChange: () => check.promise, + onValueCommitted, + }); + change(input); + fireEvent.keyDown(input, { key: "Enter" }); + fireEvent.keyDown(input, { key: "Enter" }); + expect(input.value).toBe("Changed"); + expect(input.closest("td")?.getAttribute("aria-busy")).toBe("true"); + expect(onValueCommitted).not.toHaveBeenCalled(); + if (outcome === "resolve") { + await resolveRequest(check); + expect(onValueCommitted).toHaveBeenCalledExactlyOnceWith(ROWS[0], "Changed"); + expect(input.value).toBe("Original"); + expect(input.readOnly).toBe(true); + expect(queryErrorTrigger()).toBeNull(); + } else { + await rejectRequest(check, new Error("Draft check failed")); + expect(onValueCommitted).not.toHaveBeenCalled(); + expect(input.value).toBe("Changed"); + expect(input.readOnly).toBe(false); + expect(input.getAttribute("aria-invalid")).toBe("true"); + } + expect(input.closest("td")?.getAttribute("aria-busy")).toBeNull(); + }, + ); + + it("deduplicates Enter and blur while confirming, then clears the draft only on success", async () => { + const save = deferred(); + const onValueCommitted = vi.fn(() => save.promise); + const { input, user } = renderEditor({ onValueCommitted }); + change(input); + fireEvent.keyDown(input, { key: "Enter" }); + fireEvent.keyDown(input, { key: "Enter" }); + const outside = screen.getByRole("button", { name: "Outside" }); + await user.click(outside); + expect(document.activeElement).toBe(outside); + expect(onValueCommitted).toHaveBeenCalledExactlyOnceWith(ROWS[0], "Changed"); + expect(input.value).toBe("Changed"); + expect(input.readOnly).toBe(false); + expect(input.closest("td")?.getAttribute("aria-busy")).toBe("true"); + await resolveRequest(save); + expect(input.value).toBe("Original"); + expect(input.readOnly).toBe(true); + expect(input.closest("td")?.getAttribute("aria-busy")).toBeNull(); + expect(document.activeElement).toBe(outside); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + }); + + it("retains a failed confirmation through blur and refocus, then permits an Enter retry", async () => { + let rejected = true; + const onValueCommitted = vi.fn(() => { + if (rejected) throw new Error("Temporary save failure"); + }); + const { input, user } = renderEditor({ onValueCommitted }); + change(input); + fireEvent.keyDown(input, { key: "Enter" }); + await user.click(screen.getByRole("button", { name: "Outside" })); + expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + await user.click(input); + expect(input.value).toBe("Changed"); + rejected = false; + fireEvent.keyDown(input, { key: "Enter" }); + expect(onValueCommitted).toHaveBeenCalledTimes(2); + expect(onValueCommitted).toHaveBeenLastCalledWith(ROWS[0], "Changed"); + expect(input.value).toBe("Original"); + expect(input.readOnly).toBe(true); + expect(input.getAttribute("aria-invalid")).toBeNull(); + expect(queryErrorTrigger()).toBeNull(); + }); + + it("retains the exact numeric draft rather than its normalized rejected output", () => { + const onValueCommitted = vi.fn(() => { + throw new Error("Quantity rejected"); + }); + renderColumn({ id: "value", label: "Value", type: "number", edit: { onValueCommitted } }, [ + { id: "1", value: "12" }, + ]); + const input = screen.getByRole("textbox", { name: "Value" }) as HTMLInputElement; + change(input, "１３.５０"); + fireEvent.keyDown(input, { key: "Enter" }); + expect(onValueCommitted).toHaveBeenCalledExactlyOnceWith({ id: "1", value: "12" }, 13.5); + expect(input.value).toBe("１３.５０"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + fireEvent.keyDown(input, { key: "Escape" }); + expect(input.value).toBe("12"); + expect(input.getAttribute("aria-invalid")).toBeNull(); + expect(queryErrorTrigger()).toBeNull(); + }); + + it("reverts a failed draft and error together to the latest consumer data", async () => { + const onValueChange = vi.fn(); + const onValueCommitted = vi.fn(() => { + throw new Error("Not accepted"); + }); + const { input, user, updateRows } = renderEditor({ onValueChange, onValueCommitted }); + change(input); + fireEvent.keyDown(input, { key: "Enter" }); + const latest = { id: "1", value: "Updated externally" }; + updateRows([latest]); + expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + await user.click(errorTrigger()); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + await user.click(within(popup).getByRole("button", { name: "Revert edit" })); + expect(input.value).toBe("Updated externally"); + expect(input.readOnly).toBe(true); + expect(input.getAttribute("aria-invalid")).toBeNull(); + expect(queryErrorTrigger()).toBeNull(); + expect(screen.queryByRole("dialog")).toBeNull(); + expect(onValueChange).toHaveBeenLastCalledWith(latest, "Updated externally"); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + }); + + it("uses the first Escape to close error details and the next Escape to cancel the failed draft", async () => { + const onValueCommitted = vi.fn(() => { + throw new Error("Not accepted"); + }); + const { input, user } = renderEditor({ onValueCommitted }); + change(input); + fireEvent.keyDown(input, { key: "Enter" }); + fireEvent.keyDown(input, { key: "ArrowDown", altKey: true }); + await screen.findByRole("dialog", { name: "Error in Value" }); + await user.keyboard("{Escape}"); + await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + await user.keyboard("{Escape}"); + expect(input.value).toBe("Original"); + expect(input.getAttribute("aria-invalid")).toBeNull(); + expect(queryErrorTrigger()).toBeNull(); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + }); + + it("clears cancellation errors and ignores pending results after Escape", async () => { + const request = deferred(); + const onValueCommitted = vi.fn(); + const { input } = renderEditor({ + onValueChange: (_row, draft) => { + if (draft === "Original") throw new Error("Even restoring can fail"); + return request.promise; + }, + onValueCommitted, + }); + change(input); + fireEvent.keyDown(input, { key: "Escape" }); + await rejectRequest(request, new Error("Cancelled failure")); + expect(input.value).toBe("Original"); + expect(queryErrorTrigger()).toBeNull(); + expect(onValueCommitted).not.toHaveBeenCalled(); + }); + + it("discards errors on unmount without leaking a late rejection into another cell instance", async () => { + const request = deferred(); + const view = renderEditor({ onValueChange: () => request.promise, onValueCommitted: noop }); + change(view.input); + view.unmount(); + const { input } = renderEditor({ onValueCommitted: noop }); + await rejectRequest(request, new Error("Unmounted failure")); + expect(input.value).toBe("Original"); + expect(queryErrorTrigger()).toBeNull(); + }); + + it("keeps failed drafts cell-local and allows Tab navigation without confirming them", () => { + const onValueCommitted = vi.fn(); + const { inputs } = renderEditor( + { + onValueChange: () => { + throw new Error("Draft failure"); + }, + onValueCommitted, + }, + { rows: [...ROWS, { id: "2", value: "Second" }] }, + ); + change(inputs[0]); + expect(inputs[0].getAttribute("aria-invalid")).toBe("true"); + expect(inputs[1].getAttribute("aria-invalid")).toBeNull(); + fireEvent.keyDown(inputs[0], { key: "Tab" }); + expect(onValueCommitted).not.toHaveBeenCalled(); + expect(document.activeElement).toBe(inputs[1]); + expect(inputs[0].value).toBe("Changed"); + expect(inputs[0].getAttribute("aria-invalid")).toBe("true"); + expect(queryErrorTrigger()).not.toBeNull(); + act(() => inputs[0].focus()); + expect(inputs[0].value).toBe("Changed"); + expect(inputs[0].getAttribute("aria-invalid")).toBe("true"); + }); + + it.each(["text", "badge"] as const)( + "retains a rejected %s choice through error inspection until cancellation", + async (type) => { + const onValueCommitted = vi.fn(() => { + throw new Error("Choice rejected"); + }); + const user = renderColumn({ + id: "value", + label: "Value", + type, + edit: { + options: [ + { value: "Original", label: "Original" }, + { value: "Changed", label: "Changed" }, + ], + onValueCommitted, + }, + }); + const input = screen.getByRole("combobox", { name: "Value" }) as HTMLInputElement; + await user.click(input); + await user.click(await screen.findByRole("option", { name: "Changed" })); + await user.click(errorTrigger()); + expect(within(await screen.findByRole("dialog")).getByText("Choice rejected")).toBeDefined(); + await user.keyboard("{Escape}"); + await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + expect(input.value).toBe("Changed"); + expect(input.readOnly).toBe(false); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + fireEvent.keyDown(input, { key: "Escape" }); + expect(input.value).toBe("Original"); + expect(input.readOnly).toBe(true); + expect(queryErrorTrigger()).toBeNull(); + }, + ); + + it("does not confirm a selected choice until its async change check succeeds", async () => { + const check = deferred(); + const onValueCommitted = vi.fn(); + const user = renderColumn({ + id: "value", + label: "Value", + type: "text", + edit: { + options: [ + { value: "Original", label: "Original" }, + { value: "Changed", label: "Changed" }, + ], + onValueChange: () => check.promise, + onValueCommitted, + }, + }); + const input = screen.getByRole("combobox", { name: "Value" }) as HTMLInputElement; + await user.click(input); + await user.click(await screen.findByRole("option", { name: "Changed" })); + expect(input.value).toBe("Changed"); + expect(onValueCommitted).not.toHaveBeenCalled(); + expect(input.closest("td")?.getAttribute("aria-busy")).toBe("true"); + await resolveRequest(check); + expect(onValueCommitted).toHaveBeenCalledExactlyOnceWith(ROWS[0], "Changed"); + expect(input.value).toBe("Original"); + expect(input.readOnly).toBe(true); + expect(input.closest("td")?.getAttribute("aria-busy")).toBeNull(); + }); + + it("retains a failed date candidate outside its calendar until the edit is reverted", async () => { + const onValueCommitted = vi.fn(() => { + throw new Error("Date rejected"); + }); + const user = renderColumn( + { + id: "value", + label: "Value", + type: "date", + edit: { onValueCommitted }, + }, + [{ id: "1", value: "2026-10-02" }], + ); + const trigger = screen.getByRole("button", { name: /^Value/ }); + const initialLabel = trigger.getAttribute("aria-label"); + await user.click(trigger); + const calendar = document.querySelector('[data-slot="data-table-cell-calendar"]')!; + const day = within(calendar) + .getAllByRole("button") + .find((button) => button.textContent === "20" && !button.dataset.outsideMonth)!; + await user.click(day); + expect(onValueCommitted).toHaveBeenCalledExactlyOnceWith( + { id: "1", value: "2026-10-02" }, + "2026-10-20", + ); + await user.click(document.body); + expect(trigger.getAttribute("aria-label")).toBe("Value, 2026-10-20"); + expect(trigger.closest("td")?.textContent).toContain("2026-10-20"); + expect(trigger.getAttribute("aria-invalid")).toBe("true"); + await user.click(errorTrigger()); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + await user.click(within(popup).getByRole("button", { name: "Revert edit" })); + expect(trigger.getAttribute("aria-label")).toBe(initialLabel); + expect(trigger.getAttribute("aria-invalid")).toBeNull(); + expect(queryErrorTrigger()).toBeNull(); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + }); + + it("does not bypass a failed date change check by clearing the same draft twice", async () => { + const onValueChange = vi.fn(() => { + throw new Error("A date is required"); + }); + const onValueCommitted = vi.fn(); + const user = renderColumn( + { id: "value", label: "Value", type: "date", edit: { onValueChange, onValueCommitted } }, + [{ id: "1", value: "2026-10-02" }], + ); + await user.click(screen.getByRole("button", { name: /^Value/ })); + const clear = screen.getByRole("button", { name: "Clear" }); + await user.click(clear); + await user.click(clear); + expect(onValueChange).toHaveBeenCalledTimes(1); + expect(onValueCommitted).not.toHaveBeenCalled(); + expect(screen.getByRole("button", { name: "Value, —" }).getAttribute("aria-invalid")).toBe( + "true", + ); + }); + + it("waits for the date change check when the same Clear action is repeated", async () => { + const check = deferred(); + const onValueChange = vi.fn(() => check.promise); + const onValueCommitted = vi.fn(); + const user = renderColumn( + { id: "value", label: "Value", type: "date", edit: { onValueChange, onValueCommitted } }, + [{ id: "1", value: "2026-10-02" }], + ); + await user.click(screen.getByRole("button", { name: /^Value/ })); + const clear = screen.getByRole("button", { name: "Clear" }); + await user.click(clear); + await user.click(clear); + expect(onValueChange).toHaveBeenCalledTimes(1); + expect(onValueCommitted).not.toHaveBeenCalled(); + await resolveRequest(check); + expect(onValueCommitted).toHaveBeenCalledExactlyOnceWith( + { id: "1", value: "2026-10-02" }, + null, + ); + }); + + it("does not duplicate a pending date confirmation when Clear is repeated", async () => { + const request = deferred(); + const onValueCommitted = vi.fn(() => request.promise); + const user = renderColumn( + { id: "value", label: "Value", type: "date", edit: { onValueCommitted } }, + [{ id: "1", value: "2026-10-02" }], + ); + await user.click(screen.getByRole("button", { name: /^Value/ })); + const clear = screen.getByRole("button", { name: "Clear" }); + await user.click(clear); + await user.click(clear); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + await rejectRequest(request, new Error("Clear failed")); + expect(screen.getByRole("button", { name: "Value, —" }).getAttribute("aria-invalid")).toBe( + "true", + ); + }); + + it("preserves a date-time draft while inspecting its error and clears it on cancellation", async () => { + const onValueCommitted = vi.fn(); + const user = renderColumn({ + id: "value", + label: "Value", + type: "date", + typeOptions: { dateFormat: "datetime" }, + edit: { + onValueChange: () => { + throw new Error("Time rejected"); + }, + onValueCommitted, + }, + }); + await user.click(screen.getByRole("button", { name: /^Value/ })); + const time = screen.getByLabelText("Value (Choose time)") as HTMLInputElement; + fireEvent.change(time, { target: { value: "14:30" } }); + await user.click(errorTrigger()); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + expect(within(popup).getByText("Time rejected")).toBeDefined(); + expect(time.isConnected).toBe(true); + expect(time.value).toBe("14:30"); + expect(onValueCommitted).not.toHaveBeenCalled(); + await user.keyboard("{Escape}"); + await vi.waitFor(() => + expect(screen.queryByRole("dialog", { name: "Error in Value" })).toBeNull(), + ); + expect(time.value).toBe("14:30"); + await user.click(time); + await user.keyboard("{Escape}"); + expect(queryErrorTrigger()).toBeNull(); + expect(onValueCommitted).not.toHaveBeenCalled(); + }); + + it("finishes eagerly and leaves promise failures to the consumer when error:false", async () => { + const request = deferred(); + const handled = request.promise.catch(noop); + const onValueCommitted = vi.fn(() => request.promise); + const { input } = renderEditor({ onValueCommitted, error: false }); + change(input); + fireEvent.keyDown(input, { key: "Enter" }); + expect(onValueCommitted).toHaveBeenCalledExactlyOnceWith(ROWS[0], "Changed"); + expect(input.value).toBe("Original"); + expect(input.readOnly).toBe(true); + expect(input.closest("td")?.getAttribute("aria-busy")).toBeNull(); + await act(async () => { + request.reject(new Error("Consumer-owned failure")); + await handled; + }); + expect(queryErrorTrigger()).toBeNull(); + }); +}); + +it("types error:false and custom content without empty, true, presentation, or cellContent options", () => { + const disabled: TextCellEditOptions = { onValueCommitted: noop, error: false }; + const custom: TextCellEditOptions = { + onValueCommitted: async (_row, value) => { + expectTypeOf(value).toEqualTypeOf(); + }, + error: { + render: (ctx) => { + expectTypeOf(ctx.row).toEqualTypeOf(); + expectTypeOf(ctx.error).toEqualTypeOf(); + // @ts-expect-error — inline presentation is not part of this API + expect(ctx.cellContent).toBeUndefined(); + return

Custom error

; + }, + }, + }; + const empty: TextCellEditOptions = { + onValueCommitted: noop, + // @ts-expect-error — custom settings require render + error: {}, + }; + const enabled: TextCellEditOptions = { + onValueCommitted: noop, + // @ts-expect-error — standard errors are enabled by default, not error:true + error: true, + }; + const presentation: TextCellEditOptions = { + onValueCommitted: noop, + error: { + // @ts-expect-error — only popover presentation is supported, with no type option + type: "inline", + render: () => "ERROR", + }, + }; + expect(disabled.error).toBe(false); + expect(custom.error).toBeDefined(); + expect(empty.error).toEqual({}); + expect(enabled.error).toBe(true); + expect(presentation.error).toBeDefined(); +}); diff --git a/packages/core/src/components/data-table/editable-cell.tsx b/packages/core/src/components/data-table/editable-cell.tsx index 1e1db45d0..5784d9521 100644 --- a/packages/core/src/components/data-table/editable-cell.tsx +++ b/packages/core/src/components/data-table/editable-cell.tsx @@ -10,12 +10,13 @@ import { type KeyboardEvent, type MouseEvent, type ReactNode, + type RefObject, type TouchEvent, } from "react"; import { Combobox as BaseCombobox } from "@base-ui/react/combobox"; import { Popover } from "@base-ui/react/popover"; import type { CalendarDate } from "@internationalized/date"; -import { CalendarDays, ChevronDown } from "lucide-react"; +import { CalendarDays, ChevronDown, CircleAlert } from "lucide-react"; import { cn } from "@/lib/utils"; import { useTimeZone } from "@/contexts/appshell-context"; import { BadgeList } from "@/components/badge-list"; @@ -42,6 +43,7 @@ import { } from "./cell-edit"; import type { CellEditNavigation } from "./use-cell-edit-navigation"; import { useDataTableT } from "./i18n"; +import { useCellEditSession, type CellEditFailure } from "./use-cell-edit-session"; /** * A column whose cells can be edited in place. @@ -123,7 +125,7 @@ const CELL_FOCUS_CLASS_NAME = cn( "astw:focus-visible:ring-2 astw:focus-visible:ring-inset astw:focus-visible:ring-primary", ); -// Only malformed numeric input is handled here; business errors belong to the consumer. +// Parsing and callback errors share the cell's error outline. const CELL_ERROR_CLASS_NAME = "astw:ring-2 astw:ring-inset astw:ring-destructive astw:focus:ring-destructive astw:focus-visible:ring-destructive"; @@ -168,9 +170,20 @@ const CELL_ICON_CLASS_NAME = cn( "astw:[&_svg]:size-3.5", ); +interface CellErrorUI { + failure: CellEditFailure; + id: string; + message: string; + content: ReactNode; + triggerRef: RefObject; + popupRef: RefObject; + containsTarget: (target: EventTarget | null) => boolean; +} + function useCellValue>( row: TRow, col: EditableColumn, + failure: CellEditFailure | undefined, dateRenderer?: (row: TRow) => ReactNode, ) { const { value: timeZone } = useTimeZone(); @@ -179,13 +192,39 @@ function useCellValue>( const display = render ? render(row) : renderTypedValue(row, col, current, { linkAsText: true, timeZone }); - const change = (value: unknown) => { - (col.edit?.onValueChange as ((row: TRow, value: unknown) => void) | undefined)?.(row, value); - }; - const commit = (value: unknown) => { - (col.edit?.onValueCommitted as ((row: TRow, value: unknown) => void) | undefined)?.(row, value); - }; - return { current, display, change, commit }; + const t = useDataTableT(); + const errorId = useId(); + const triggerRef = useRef(null); + const popupRef = useRef(null); + const containsErrorTarget = (target: EventTarget | null) => + target instanceof Node && + !!(triggerRef.current?.contains(target) || popupRef.current?.contains(target)); + const change = (value: unknown) => + (col.edit?.onValueChange as ((row: TRow, value: unknown) => unknown) | undefined)?.(row, value); + const commit = (value: unknown) => + (col.edit?.onValueCommitted as ((row: TRow, value: unknown) => unknown) | undefined)?.( + row, + value, + ); + const errorConfig = col.edit?.error; + const renderError = errorConfig && errorConfig.render; + const message = + failure?.error instanceof Error && failure.error.message + ? failure.error.message + : t("editCallbackError"); + const callbackError: CellErrorUI | undefined = + failure && failure.source !== "parse" + ? { + failure, + id: errorId, + message: renderError ? t("editCallbackError") : message, + content: renderError ? renderError({ row, error: failure.error }) : message, + triggerRef, + popupRef, + containsTarget: containsErrorTarget, + } + : undefined; + return { current, display, change, commit, callbackError, containsErrorTarget }; } /** @@ -217,6 +256,11 @@ function EditableCellFrame({ icon, tooltipLabel, editing, + callbackError, + label, + onEditorLeave, + onEditorCancel, + confirming, children, }: { cellProps: ComponentProps; @@ -224,21 +268,56 @@ function EditableCellFrame({ truncate?: boolean; tooltipLabel?: string; editing: boolean; + callbackError?: CellErrorUI; + label: string; + onEditorLeave?: () => void; + onEditorCancel: () => void; + confirming: boolean; /** Typed-into cells hide the display while the input shows the text. */ hideDisplay?: "focused" | "forced-colors"; /** Shown on hover or focus; the cell's tint marks editability at rest. */ icon?: ReactNode; children: ReactNode; }) { + const t = useDataTableT(); const [tooltipOpen, setTooltipOpen] = useState(false); - const showTooltip = tooltipLabel !== undefined && !editing; + const [openFailure, setOpenFailure] = useState(null); + const showTooltip = tooltipLabel !== undefined && !editing && !callbackError; const cell = ( { + if ( + event.currentTarget.contains(event.relatedTarget) || + callbackError?.containsTarget(event.relatedTarget) + ) + return; + onEditorLeave?.(); + }} + onKeyDownCapture={(event) => { + if (callbackError && event.key === "Escape" && openFailure !== callbackError.failure) { + event.preventDefault(); + event.stopPropagation(); + onEditorCancel(); + return; + } + if (callbackError && event.altKey && event.key === "ArrowDown") { + event.preventDefault(); + event.stopPropagation(); + setOpenFailure(callbackError.failure); + } + }} // Editing a cell must never fire `onClickRow`. onClick={(event) => event.stopPropagation()} > @@ -246,7 +325,7 @@ function EditableCellFrame({ aria-hidden="true" className={cn( "astw:block", - icon !== undefined && ICON_SPACE_CLASS_NAME, + (icon !== undefined || callbackError) && ICON_SPACE_CLASS_NAME, truncate && "astw:truncate", // Transparent input text is forced visible in forced-colors mode; // hide the display instead so the two don't overlap. @@ -257,7 +336,49 @@ function EditableCellFrame({ {display} {children} - {icon !== undefined && ( + {callbackError && ( + <> + + {callbackError.message}. {t("editErrorHint")} + + setOpenFailure(open ? callbackError.failure : null)} + > + event.stopPropagation()} + > + + + + event.stopPropagation()} + onTouchStart={(event) => event.stopPropagation()} + onKeyDown={(event) => { + if (event.key === "Escape") event.stopPropagation(); + }} + > + {callbackError.content} + + + + + + + )} + {icon !== undefined && !callbackError && ( @@ -308,11 +429,12 @@ export function DataTableEditableCell>( props: DataTableEditableCellProps, ) { const { col } = props; - if (col.type === "date") return ; + const mode = col.edit?.error === false ? "unhandled" : "handled"; + if (col.type === "date") return ; if (col.type === "badge" || ((col.type === "text" || col.type === "link") && col.edit?.options)) { - return ; + return ; } - return ; + return ; } // ── Typing: text, number, money, link ─────────────────────────────────────── @@ -330,21 +452,18 @@ function TypingEditCell>({ }: DataTableEditableCellProps) { const t = useDataTableT(); const errorId = useId(); - const { current, display, change, commit } = useCellValue(row, col); + const session = useCellEditSession(col.edit?.error !== false); + const { state } = session; + const failure = state.status === "failed" ? state : undefined; + const { current, display, change, commit, callbackError, containsErrorTarget } = useCellValue( + row, + col, + failure, + ); const register = useNavigationRef(navigation, rowKey, colKey); - - const [draft, setDraftState] = useState(null); - // Read by handlers that run before the next render: the blur fired by moving - // focus right after Enter must see that Enter already committed the draft. - const draftRef = useRef(draft); - const setDraft = (next: string | null) => { - draftRef.current = next; - setDraftState(next); - }; + const editing = state.status !== "idle"; const [focused, setFocused] = useState(false); - const [editing, setEditing] = useState(false); const [hovered, setHovered] = useState(false); - const [showParseError, setShowParseError] = useState(false); const composingRef = useRef(false); const selectOnMouseUpRef = useRef(false); @@ -352,27 +471,27 @@ function TypingEditCell>({ const baselineText = numeric ? toNumberEditText(toNumberValue(current)) : (toTextValue(current) ?? ""); - const text = draft ?? baselineText; - const malformed = - numeric && draft !== null && parseNumberText(normalizeNumberText(draft)) === undefined; - const message = showParseError && malformed ? t("editNotANumber") : undefined; + const text = state.status === "idle" ? baselineText : state.draft; + const message = failure?.source === "parse" ? t("editNotANumber") : undefined; const revert = (notify = false) => { - const previous = draftRef.current; - setDraft(null); - setShowParseError(false); - if (notify && previous !== null && previous !== baselineText) change(baselineText); + const previous = session.getState(); + session.cancel( + notify && previous.status !== "idle" && previous.draft !== baselineText + ? () => change(baselineText) + : undefined, + ); }; - // Clear the draft before notifying: moving focus immediately fires blur. const commitDraft = (): boolean => { - const draftText = draftRef.current; - if (draftText === null) return true; + const active = session.getState(); + if (active.status === "idle" || active.status === "confirming") return true; + const draftText = active.draft; const value = numeric ? parseNumberText(normalizeNumberText(draftText)) : toTextValue(draftText); if (value === undefined) { - setShowParseError(true); + session.fail(undefined, "parse"); return false; } const unchanged = @@ -380,16 +499,15 @@ function TypingEditCell>({ (numeric ? sameNumber(value as number | null, toNumberValue(current)) : value === toTextValue(current)); - revert(); - if (!unchanged) commit(value); + session.confirm(() => (unchanged ? undefined : commit(value))); return true; }; const handleChange = (event: ChangeEvent) => { const next = event.target.value; - const previous = draftRef.current ?? baselineText; - setDraft(next); - if (next !== previous) change(next); + const active = session.getState(); + const previous = active.status === "idle" ? baselineText : active.draft; + if (next !== previous) session.change(next, () => change(next)); }; // Keep IME output raw until cell confirmation, just like other draft changes. @@ -404,12 +522,11 @@ function TypingEditCell>({ switch (event.key) { case "Enter": event.preventDefault(); - if (!editing) { - setDraft(baselineText); - setEditing(true); + if (session.getState().status === "idle") { + session.begin(baselineText); event.currentTarget.select(); - } else if (commitDraft()) { - setEditing(false); + } else { + commitDraft(); } return; case "Tab": @@ -421,27 +538,31 @@ function TypingEditCell>({ if (navigation.move(from, event.shiftKey ? "prev" : "next")) event.preventDefault(); return; case "Escape": - if (draftRef.current === null) return; + if (session.getState().status === "idle") return; // Handled here, so an enclosing dialog doesn't also close on this Esc. event.preventDefault(); event.stopPropagation(); revert(true); - setEditing(false); return; } }; const handleFocus = (event: FocusEvent) => { + const withinCell = focused || containsErrorTarget(event.relatedTarget); setFocused(true); - setEditing(true); - setDraft(baselineText); + if (withinCell) return; + session.begin(baselineText); event.currentTarget.select(); }; - // Malformed text cannot be emitted as a number; blur restores consumer data. const handleBlur = () => { setFocused(false); - setEditing(false); + const active = session.getState(); + if (active.status === "failed") { + // Preserve callback failures; malformed numeric blur still cancels the edit. + if (active.source === "parse") revert(true); + return; + } if (!commitDraft()) revert(true); }; @@ -452,8 +573,7 @@ function TypingEditCell>({ event.preventDefault(); return; } - setDraft(baselineText); - setEditing(true); + session.begin(baselineText); if (focused) event.currentTarget.select(); selectOnMouseUpRef.current = true; }; @@ -482,6 +602,11 @@ function TypingEditCell>({ hideDisplay={editing ? "focused" : "forced-colors"} tooltipLabel={tooltipLabel} editing={editing} + callbackError={callbackError} + label={label} + onEditorLeave={handleBlur} + onEditorCancel={() => revert(true)} + confirming={state.status === "confirming"} > {message !== undefined && ( @@ -497,8 +622,9 @@ function TypingEditCell>({ value={text} readOnly={!editing} aria-label={label} - aria-invalid={message !== undefined ? true : undefined} - aria-describedby={message !== undefined ? errorId : undefined} + aria-invalid={message !== undefined || callbackError ? true : undefined} + aria-describedby={message !== undefined ? errorId : callbackError?.id} + aria-keyshortcuts={callbackError ? "Alt+ArrowDown" : undefined} inputMode={numeric ? "text" : undefined} enterKeyHint="done" autoComplete="off" @@ -508,13 +634,13 @@ function TypingEditCell>({ !editing && "astw:text-transparent", numeric && "astw:tabular-nums", align === "right" && "astw:text-right", - message !== undefined && CELL_ERROR_CLASS_NAME, + (message !== undefined || callbackError) && CELL_ERROR_CLASS_NAME, + callbackError && ICON_INPUT_CLASS_NAME, )} style={editing ? { WebkitTouchCallout: "default" } : undefined} onChange={handleChange} onKeyDown={handleKeyDown} onFocus={handleFocus} - onBlur={handleBlur} onCompositionStart={() => { composingRef.current = true; }} @@ -559,6 +685,8 @@ interface Choice { label: string; } +type ChoiceDraft = Choice & { notified?: boolean }; + // Case- and width-insensitive searching: full-width letters and half-width // katakana match their usual forms. const searchable = (text: string) => text.normalize("NFKC").toLocaleLowerCase().trim(); @@ -579,7 +707,13 @@ function ChoiceEditCell>({ tooltipLabel, }: DataTableEditableCellProps>) { const t = useDataTableT(); - const { current, display, change, commit } = useCellValue(row, col); + const session = useCellEditSession(col.edit?.error !== false); + const { state } = session; + const { current, display, change, commit, callbackError, containsErrorTarget } = useCellValue( + row, + col, + state.status === "failed" ? state : undefined, + ); const register = useNavigationRef(navigation, rowKey, colKey); const value = toTextValue(current); const noneLabel = t("editNone"); @@ -598,12 +732,8 @@ function ChoiceEditCell>({ const selected = items.find((item) => item.value === value) ?? null; const [open, setOpen] = useState(false); const [focused, setFocused] = useState(false); - const [editing, setEditingState] = useState(false); - const editingRef = useRef(false); - const setEditing = (next: boolean) => { - editingRef.current = next; - setEditingState(next); - }; + const editing = state.status !== "idle"; + const activeChoice = state.status === "idle" ? selected : state.draft; // The displayed search text is local while editing, even if consumer data changes. const [query, setQuery] = useState(null); // Keep the closing list filtered until its animation completes. @@ -631,27 +761,44 @@ function ChoiceEditCell>({ }, [items, listQuery]); const startEditing = () => { - setEditing(true); + session.begin(selected ?? { value, label: value ?? noneLabel }); setQuery(selected?.label ?? ""); setListQuery(null); highlightedRef.current = undefined; explicitHighlightRef.current = undefined; }; - const finishEditing = () => { - // Clear synchronous state before closing or moving focus can fire blur again. - setEditing(false); + const closeEditor = () => { setQuery(null); explicitHighlightRef.current = undefined; setOpen(false); }; + const cancelEditing = () => { + const active = session.getState(); + session.cancel( + active.status !== "idle" && active.draft.notified && active.draft.value !== value + ? () => change(value) + : undefined, + ); + closeEditor(); + }; + + const leaveEditor = () => { + const active = session.getState(); + if (active.status === "failed" || active.status === "confirming") closeEditor(); + else cancelEditing(); + }; + const pick = (item: Choice) => { - finishEditing(); - if (item.value !== value) { - change(item.value); - commit(item.value); - } + const active = session.getState(); + if (active.status === "confirming" && active.draft.value === item.value) return; + session.change( + { ...item, notified: item.value !== value }, + item.value !== value ? () => change(item.value) : undefined, + ); + setQuery(item.label); + session.confirm(() => (item.value !== value ? commit(item.value) : undefined), closeEditor); }; const explicitCandidate = () => { @@ -678,55 +825,66 @@ function ChoiceEditCell>({ case "Enter": { event.preventDefault(); event.preventBaseUIHandler(); + const active = session.getState(); + if (active.status === "confirming") return; if (!open) { - if (!editingRef.current) startEditing(); + if (active.status === "failed" && active.source === "commit") { + session.confirm( + () => (active.draft.value !== value ? commit(active.draft.value) : undefined), + closeEditor, + ); + return; + } + if (active.status === "idle") startEditing(); setOpen(true); return; } const item = keyboardCandidate(); if (item) pick(item); - else finishEditing(); + else leaveEditor(); return; } case "Tab": { event.preventBaseUIHandler(); const item = - editingRef.current && open + session.getState().status !== "idle" && open ? (explicitCandidate() ?? (searchable(listQuery ?? "") ? keyboardCandidate() : undefined)) : undefined; if (item) pick(item); - else finishEditing(); + else leaveEditor(); if (navigation.move(from, event.shiftKey ? "prev" : "next")) event.preventDefault(); return; } case "Escape": - if (!editingRef.current && !open) return; + if (session.getState().status === "idle" && !open) return; event.preventDefault(); event.stopPropagation(); event.preventBaseUIHandler(); - finishEditing(); + cancelEditing(); return; } }; const handleFocus = (event: FocusEvent) => { + const withinCell = focused || containsErrorTarget(event.relatedTarget); setFocused(true); - if (!editingRef.current) startEditing(); + if (withinCell) return; + if (session.getState().status === "idle") startEditing(); event.currentTarget.select(); }; const handleBlur = () => { setFocused(false); - finishEditing(); + leaveEditor(); }; const handleMouseDown = (event: MouseEvent) => { - if (event.button === 2 && !editingRef.current) { + if (event.button === 2 && session.getState().status === "idle") { event.preventDefault(); return; } - if (!editingRef.current) { + if (session.getState().status === "idle") { startEditing(); setOpen(true); } @@ -740,7 +898,7 @@ function ChoiceEditCell>({ }; const stopWhileEditing = (event: MouseEvent | TouchEvent) => { - if (editingRef.current) event.stopPropagation(); + if (session.getState().status !== "idle") event.stopPropagation(); }; const renderChoice = (item: Choice) => { @@ -761,17 +919,28 @@ function ChoiceEditCell>({ icon={} tooltipLabel={tooltipLabel} editing={editing} + callbackError={callbackError} + label={label} + onEditorLeave={handleBlur} + onEditorCancel={cancelEditing} + confirming={state.status === "confirming"} > items={items} filteredItems={filtered} - value={selected} + value={activeChoice} onValueChange={(item, details) => { if (details.reason === "item-press" && item) pick(item); }} - inputValue={query ?? selected?.label ?? ""} + inputValue={query ?? activeChoice?.label ?? ""} onInputValueChange={(next, details) => { if (details.reason === "input-change" || details.reason === "input-clear") { + const active = session.getState(); + session.change( + active.status === "idle" + ? (selected ?? { value, label: value ?? noneLabel }) + : active.draft, + ); setQuery(next); setListQuery(next); explicitHighlightRef.current = undefined; @@ -780,17 +949,26 @@ function ChoiceEditCell>({ open={open} onOpenChange={(next, details) => { if (next) { - if (!editingRef.current) startEditing(); + if (session.getState().status === "idle") startEditing(); setOpen(true); if (details.reason !== "input-change") setListQuery(null); return; } + if ( + containsErrorTarget(details.event.target) || + ("relatedTarget" in details.event && + containsErrorTarget(details.event.relatedTarget as EventTarget | null)) + ) { + details.cancel(); + return; + } // Clearing search text is not selection or cancellation. if (details.reason === "input-clear") { setOpen(false); return; } - finishEditing(); + if (details.reason === "escape-key") cancelEditing(); + else leaveEditor(); }} onOpenChangeComplete={(isOpen) => { if (!isOpen) setListQuery(null); @@ -808,6 +986,9 @@ function ChoiceEditCell>({ >({ ICON_INPUT_CLASS_NAME, !editing && "astw:text-transparent astw:cursor-pointer", align === "right" && "astw:text-right", + callbackError && CELL_ERROR_CLASS_NAME, )} onKeyDown={handleKeyDown} onFocus={handleFocus} - onBlur={handleBlur} onMouseDown={handleMouseDown} onMouseUp={handleMouseUp} onContextMenu={stopWhileEditing} @@ -869,77 +1050,120 @@ function DateEditCell>({ const t = useDataTableT(); const { value: timeZone } = useTimeZone(); const withTime = col.typeOptions?.dateFormat === "datetime"; - const { current, display, change, commit } = useCellValue(row, col, dateRenderer); + const session = useCellEditSession(col.edit?.error !== false); + const { state } = session; + const { current, display, change, commit, callbackError, containsErrorTarget } = useCellValue( + row, + col, + state.status === "failed" ? state : undefined, + dateRenderer, + ); const register = useNavigationRef(navigation, rowKey, colKey); const baseline: DateTimeDraft = { day: toCalendarDate(current, timeZone)?.toString() ?? null, time: toTimeText(current, timeZone) || null, }; const [open, setOpen] = useState(false); - const [draft, setDraftState] = useState(null); - // Close notifications can run before the state update from a pick is rendered. - const draftRef = useRef(null); const differs = (a: DateTimeDraft, b: DateTimeDraft) => a.day !== b.day || (withTime && a.time !== b.time); const notifyDraft = (next: DateTimeDraft) => change(withTime ? { ...next } : next.day); const setDraft = (next: DateTimeDraft) => { - const previous = draftRef.current ?? baseline; - draftRef.current = next; - setDraftState(next); - if (differs(previous, next)) notifyDraft(next); + const active = session.getState(); + const previous = active.status === "idle" ? baseline : active.draft; + if (!differs(previous, next)) { + session.begin(next); + return; + } + session.change(next, () => notifyDraft(next)); }; - const discardDraft = () => { - draftRef.current = null; - setDraftState(null); + const cancelEditing = () => { + const active = session.getState(); + session.cancel( + active.status !== "idle" && differs(active.draft, baseline) + ? () => notifyDraft(baseline) + : undefined, + ); + setOpen(false); }; const finish = (next: string | null) => { - discardDraft(); - setOpen(false); - if (!sameDate(next, current, withTime, timeZone)) commit(next); + session.confirm( + () => (sameDate(next, current, withTime, timeZone) ? undefined : commit(next)), + () => setOpen(false), + ); }; - const activeDraft = draft ?? baseline; + const activeDraft = state.status === "idle" ? baseline : state.draft; const pickedDay = toCalendarDate(activeDraft.day, timeZone); const timestamp = withTime && pickedDay && activeDraft.time ? toIsoDateTime(pickedDay, activeDraft.time, timeZone) : undefined; + const draftDisplay = withTime + ? `${activeDraft.day ?? "—"} ${activeDraft.time ?? "—"}` + : (activeDraft.day ?? "—"); + const visibleDisplay = state.status === "idle" ? display : draftDisplay; // The trigger's accessible name carries the date hidden in the visual display. - const accessibleName = typeof display === "string" ? `${label}, ${display}` : label; - - const handleOpenChange = (next: boolean) => { + const accessibleName = typeof visibleDisplay === "string" ? `${label}, ${visibleDisplay}` : label; + + const handleOpenChange = (next: boolean, details?: Popover.Root.ChangeEventDetails) => { + if ( + !next && + details && + (containsErrorTarget(details.event.target) || + ("relatedTarget" in details.event && + containsErrorTarget(details.event.relatedTarget as EventTarget | null))) + ) { + details.cancel(); + return; + } if (next) { - if (draftRef.current === null) { - draftRef.current = baseline; - setDraftState(baseline); - } + session.begin(baseline); } else { - const previous = draftRef.current; - discardDraft(); - if (previous && differs(previous, baseline)) notifyDraft(baseline); + const active = session.getState(); + if ( + details?.reason === "escape-key" || + (active.status !== "failed" && active.status !== "confirming") + ) { + cancelEditing(); + return; + } } setOpen(next); }; const handlePick = (day: CalendarDate) => { - setDraft({ ...(draftRef.current ?? baseline), day: day.toString() }); + const active = session.getState(); + setDraft({ ...(active.status === "idle" ? baseline : active.draft), day: day.toString() }); if (!withTime) finish(day.toString()); }; return ( } tooltipLabel={tooltipLabel} editing={open} + callbackError={callbackError} + label={label} + onEditorCancel={cancelEditing} + confirming={state.status === "confirming"} > { + if (event.key === "Escape" && !open && session.getState().status !== "idle") { + event.preventDefault(); + event.stopPropagation(); + cancelEditing(); + return; + } if (event.key === "ArrowDown" && !open) { event.preventDefault(); handleOpenChange(true); @@ -969,7 +1193,7 @@ function DateEditCell>({ value={activeDraft.time ?? ""} onChange={(event) => setDraft({ - ...(draftRef.current ?? baseline), + ...activeDraft, time: event.target.value || null, }) } diff --git a/packages/core/src/components/data-table/i18n.ts b/packages/core/src/components/data-table/i18n.ts index 864861899..f52c5a12f 100644 --- a/packages/core/src/components/data-table/i18n.ts +++ b/packages/core/src/components/data-table/i18n.ts @@ -135,10 +135,15 @@ export const dataTableLabels = defineI18nLabels({ // Inline cell editing — numeric parsing feedback editNotANumber: "Enter a number", editRevertHint: "Press Esc to undo", + editRevert: "Revert edit", editNone: "None", editNoMatches: "No matches", editClear: "Clear", editDone: "Done", + editCallbackError: "An error occurred", + editShowError: (props: { label: string }) => `Show error for ${props.label}`, + editErrorDialog: (props: { label: string }) => `Error in ${props.label}`, + editErrorHint: "Press Alt+ArrowDown for error details", }, ja: { loading: "読み込み中...", @@ -257,10 +262,15 @@ export const dataTableLabels = defineI18nLabels({ // Inline cell editing editNotANumber: "数値を入力してください", editRevertHint: "Escキーで元に戻せます", + editRevert: "編集を元に戻す", editNone: "なし", editNoMatches: "一致する項目がありません", editClear: "クリア", editDone: "完了", + editCallbackError: "エラーが発生しました", + editShowError: (props: { label: string }) => `${props.label}のエラーを表示`, + editErrorDialog: (props: { label: string }) => `${props.label}のエラー`, + editErrorHint: "Alt+↓キーでエラーの詳細を表示できます", }, }); diff --git a/packages/core/src/components/data-table/types.ts b/packages/core/src/components/data-table/types.ts index 01b230ada..9e9090628 100644 --- a/packages/core/src/components/data-table/types.ts +++ b/packages/core/src/components/data-table/types.ts @@ -112,16 +112,32 @@ interface CellEditBase, TValue, TDraft = TV * Observes editor drafts, including incomplete input and restored drafts on * cancellation. Direct inputs emit raw strings; choices emit selected values, * not search text or highlights. Updating `data` here is outside the supported - * coordination contract. The return value is ignored. + * coordination contract. A failed change check retains its draft and blocks + * confirmation; pending change checks finish before `onValueCommitted` runs. + * Cancellation notifications cannot create a new cell error after discarding + * the draft. Pass `error: false` for unobserved callback behavior. */ onValueChange?: (row: TRow, draft: TDraft) => void; /** - * Notifies a changed, confirmed editor output, compared with current `data`. - * Empty cells confirm `null`. Confirmation does not imply business validation, - * adoption, or persistence; update `data` here to adopt it. Return values are - * ignored. Output types follow the editor, not the row field's runtime type. + * Attempts to confirm a changed editor output, compared with current `data`. + * Empty cells confirm `null`. A normal return or fulfilled Promise completes + * the edit; throw/rejection retains the draft and error for correction or retry. + * Focus may leave while pending. Update `data` here to adopt a successful value; + * DataTable does not persist or roll back consumer state. Output types follow + * the editor, not the row field's runtime type. */ onValueCommitted: (row: TRow, value: TValue | null) => void; + /** + * Callback failures use a standard error popover by default. Pass `false` to + * disable interception and retain eager edit completion, or `render` to replace + * the popover's body. Failed drafts stay visible until corrected or cancelled; + * Esc / Revert edit restores current data and clears the error together. + */ + error?: + | false + | { + render: (ctx: { row: TRow; error: unknown }) => ReactNode; + }; } /** diff --git a/packages/core/src/components/data-table/use-cell-edit-session.test.tsx b/packages/core/src/components/data-table/use-cell-edit-session.test.tsx new file mode 100644 index 000000000..829778310 --- /dev/null +++ b/packages/core/src/components/data-table/use-cell-edit-session.test.tsx @@ -0,0 +1,350 @@ +import { StrictMode, type ReactNode } from "react"; +import { act, cleanup, renderHook } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useCellEditSession } from "./use-cell-edit-session"; + +afterEach(cleanup); + +function Wrapper({ children }: { children: ReactNode }) { + return {children}; +} + +function setup(enabled = true) { + return renderHook(() => useCellEditSession(enabled), { wrapper: Wrapper }); +} + +function deferred() { + let resolve!: () => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +async function settle(request: ReturnType, failure?: { error: unknown }) { + await act(async () => { + if (failure) request.reject(failure.error); + else request.resolve(); + await request.promise.catch(() => undefined); + }); +} + +describe("useCellEditSession", () => { + it("begins only from idle and keeps the synchronous state aligned before rendering", () => { + const { result } = setup(); + const callback = vi.fn(); + act(() => { + result.current.confirm(callback); + result.current.begin("Original"); + expect(result.current.getState()).toEqual({ status: "editing", draft: "Original" }); + result.current.begin("Must not replace the draft"); + }); + expect(callback).not.toHaveBeenCalled(); + expect(result.current.state).toEqual({ status: "editing", draft: "Original" }); + act(() => result.current.cancel()); + expect(result.current.state).toEqual({ status: "idle" }); + }); + + it("finishes synchronous confirmation synchronously and ignores successful return values", () => { + const { result } = setup(); + const onSuccess = vi.fn(() => { + expect(result.current.getState()).toEqual({ status: "idle" }); + }); + act(() => { + result.current.change("Changed", () => 42); + result.current.confirm(() => false, onSuccess); + expect(result.current.getState()).toEqual({ status: "idle" }); + }); + expect(onSuccess).toHaveBeenCalledTimes(1); + expect(result.current.state).toEqual({ status: "idle" }); + }); + + it.each([new Error("Invalid"), null, undefined, false])( + "retains a draft with a change failure %j and blocks confirmation", + (error) => { + const { result } = setup(); + const commit = vi.fn(); + act(() => { + result.current.change("Invalid draft", () => { + throw error; + }); + result.current.confirm(commit); + }); + expect(commit).not.toHaveBeenCalled(); + expect(result.current.state).toEqual({ + status: "failed", + draft: "Invalid draft", + error, + source: "change", + }); + act(() => result.current.change("Corrected")); + expect(result.current.state).toEqual({ status: "editing", draft: "Corrected" }); + }, + ); + + it("awaits a pending change check before confirming and deduplicates repeated confirmation", async () => { + const change = deferred(); + const commit = deferred(); + const onCommit = vi.fn(() => commit.promise); + const duplicate = vi.fn(); + const onSuccess = vi.fn(); + const { result } = setup(); + act(() => { + result.current.change("Candidate", () => change.promise); + result.current.confirm(onCommit, onSuccess); + result.current.confirm(duplicate, duplicate); + }); + expect(result.current.state).toEqual({ status: "confirming", draft: "Candidate" }); + expect(onCommit).not.toHaveBeenCalled(); + await settle(change); + expect(onCommit).toHaveBeenCalledTimes(1); + expect(result.current.state).toEqual({ status: "confirming", draft: "Candidate" }); + act(() => result.current.confirm(duplicate)); + await settle(commit); + expect(result.current.state).toEqual({ status: "idle" }); + expect(onSuccess).toHaveBeenCalledTimes(1); + expect(duplicate).not.toHaveBeenCalled(); + }); + + it("does not confirm after a pending change check rejects", async () => { + const request = deferred(); + const error = new Error("Draft rejected"); + const commit = vi.fn(); + const { result } = setup(); + act(() => { + result.current.change("Candidate", () => request.promise); + result.current.confirm(commit); + }); + await settle(request, { error }); + expect(commit).not.toHaveBeenCalled(); + expect(result.current.state).toEqual({ + status: "failed", + draft: "Candidate", + error, + source: "change", + }); + act(() => result.current.confirm(commit)); + expect(commit).not.toHaveBeenCalled(); + }); + + it("retains a failed commit draft and allows retrying the same value", () => { + const { result } = setup(); + const error = new Error("Save failed"); + const onSuccess = vi.fn(); + act(() => { + result.current.begin("Candidate"); + result.current.confirm(() => { + throw error; + }, onSuccess); + }); + expect(result.current.state).toEqual({ + status: "failed", + draft: "Candidate", + error, + source: "commit", + }); + expect(onSuccess).not.toHaveBeenCalled(); + act(() => result.current.confirm(() => undefined, onSuccess)); + expect(result.current.state).toEqual({ status: "idle" }); + expect(onSuccess).toHaveBeenCalledTimes(1); + }); + + it("retains an asynchronously rejected commit draft instead of returning to idle", async () => { + const request = deferred(); + const { result } = setup(); + const onSuccess = vi.fn(); + act(() => { + result.current.change("Candidate"); + result.current.confirm(() => request.promise, onSuccess); + }); + expect(result.current.state).toEqual({ status: "confirming", draft: "Candidate" }); + await settle(request, { error: undefined }); + expect(result.current.state).toEqual({ + status: "failed", + draft: "Candidate", + error: undefined, + source: "commit", + }); + expect(onSuccess).not.toHaveBeenCalled(); + }); + + it.each(["resolve", "reject"] as const)( + "ignores an old commit %s after a newer draft replaces it", + async (outcome) => { + const request = deferred(); + const { result } = setup(); + const onSuccess = vi.fn(); + act(() => { + result.current.change("Older"); + result.current.confirm(() => request.promise, onSuccess); + result.current.change("Newer"); + }); + await settle(request, outcome === "reject" ? { error: new Error("Old failure") } : undefined); + expect(result.current.state).toEqual({ status: "editing", draft: "Newer" }); + expect(onSuccess).not.toHaveBeenCalled(); + }, + ); + + it("does not let an older change result replace the newer draft's failure", async () => { + const old = deferred(); + const currentError = new Error("Current failure"); + const { result } = setup(); + act(() => { + result.current.change("Older", () => old.promise); + result.current.change("Newer", () => { + throw currentError; + }); + }); + await settle(old, { error: new Error("Old failure") }); + expect(result.current.state).toEqual({ + status: "failed", + draft: "Newer", + error: currentError, + source: "change", + }); + }); + + it("keeps parse failures with their draft and invalidates pending change checks", async () => { + const request = deferred(); + const commit = vi.fn(); + const { result } = setup(); + act(() => { + result.current.change("-", () => request.promise); + result.current.fail("Not a number", "parse"); + result.current.confirm(commit); + }); + await settle(request, { error: new Error("Old check failure") }); + expect(result.current.state).toEqual({ + status: "failed", + draft: "-", + error: "Not a number", + source: "parse", + }); + expect(commit).not.toHaveBeenCalled(); + act(() => result.current.cancel()); + expect(result.current.state).toEqual({ status: "idle" }); + act(() => result.current.fail("No draft", "parse")); + expect(result.current.state).toEqual({ status: "idle" }); + }); + + it("cancels pending confirmation and prevents the waiting commit from starting", async () => { + const request = deferred(); + const commit = vi.fn(); + const { result } = setup(); + act(() => { + result.current.change("Candidate", () => request.promise); + result.current.confirm(commit); + result.current.cancel(); + }); + await settle(request); + expect(result.current.state).toEqual({ status: "idle" }); + expect(commit).not.toHaveBeenCalled(); + }); + + it.each(["sync", "async"] as const)( + "does not recreate an orphan error from a %s restoration failure", + async (mode) => { + const request = deferred(); + const error = new Error("Restoration failed"); + const { result } = setup(); + const restore = vi.fn(() => { + expect(result.current.getState()).toEqual({ status: "idle" }); + if (mode === "sync") throw error; + return request.promise; + }); + act(() => { + result.current.change("Candidate"); + result.current.cancel(restore); + }); + if (mode === "async") await settle(request, { error }); + expect(restore).toHaveBeenCalledTimes(1); + expect(result.current.state).toEqual({ status: "idle" }); + }, + ); + + it("ignores a late save result after cancellation and unmount", async () => { + const request = deferred(); + const onSuccess = vi.fn(); + const view = setup(); + act(() => { + view.result.current.change("Candidate"); + view.result.current.confirm(() => request.promise, onSuccess); + view.result.current.cancel(); + }); + view.unmount(); + await settle(request); + expect(onSuccess).not.toHaveBeenCalled(); + }); + + it("does not start a waiting commit after unmount", async () => { + const request = deferred(); + const commit = vi.fn(); + const view = setup(); + act(() => { + view.result.current.change("Candidate", () => request.promise); + view.result.current.confirm(commit); + }); + view.unmount(); + await settle(request); + expect(commit).not.toHaveBeenCalled(); + }); + + it("supports thenables without making synchronous callbacks asynchronous", async () => { + const { result } = setup(); + await act(async () => { + // eslint-disable-next-line unicorn/no-thenable -- Intentionally tests a foreign thenable. + result.current.change("Candidate", () => ({ then: (resolve: () => void) => resolve() })); + // eslint-disable-next-line unicorn/no-thenable -- Intentionally tests a foreign thenable. + result.current.confirm(() => ({ then: (resolve: () => void) => resolve() })); + }); + expect(result.current.state).toEqual({ status: "idle" }); + }); + + it("preserves callback propagation and eager completion when interception is disabled", async () => { + const { result } = setup(false); + const error = new Error("Consumer-owned failure"); + const onSuccess = vi.fn(() => { + expect(result.current.getState()).toEqual({ status: "idle" }); + }); + act(() => { + expect(() => + result.current.change("Candidate", () => { + throw error; + }), + ).toThrow(error); + }); + expect(result.current.state).toEqual({ status: "editing", draft: "Candidate" }); + act(() => { + expect(() => + result.current.confirm(() => { + throw error; + }, onSuccess), + ).toThrow(error); + }); + expect(onSuccess).toHaveBeenCalledTimes(1); + expect(result.current.state).toEqual({ status: "idle" }); + + const request = deferred(); + const handled = request.promise.catch(() => undefined); + const readThen = vi.fn(() => request.promise.then.bind(request.promise)); + const unobserved = { + // eslint-disable-next-line unicorn/no-thenable -- Any access would violate disabled interception. + get then() { + return readThen(); + }, + }; + act(() => { + result.current.change("Next", () => unobserved); + result.current.confirm(() => unobserved); + }); + expect(readThen).not.toHaveBeenCalled(); + expect(result.current.state).toEqual({ status: "idle" }); + await act(async () => { + request.reject(error); + await handled; + }); + expect(result.current.state).toEqual({ status: "idle" }); + }); +}); diff --git a/packages/core/src/components/data-table/use-cell-edit-session.ts b/packages/core/src/components/data-table/use-cell-edit-session.ts new file mode 100644 index 000000000..ab0c7da4f --- /dev/null +++ b/packages/core/src/components/data-table/use-cell-edit-session.ts @@ -0,0 +1,166 @@ +import { useEffect, useRef, useState } from "react"; + +/** @internal */ +export interface CellEditFailure { + error: unknown; + source: "parse" | "change" | "commit"; +} + +/** @internal */ +export type CellEditSessionState = + | { status: "idle" } + | { status: "editing"; draft: TDraft } + | { status: "confirming"; draft: TDraft } + | ({ status: "failed"; draft: TDraft } & CellEditFailure); + +function isPromiseLike(value: unknown): value is PromiseLike { + return typeof (value as PromiseLike | null | undefined)?.then === "function"; +} + +/** Owns the draft and its failure together; consumer data remains authoritative. @internal */ +export function useCellEditSession(enabled: boolean) { + const [state, setState] = useState>({ status: "idle" }); + const { current: session } = useRef({ + state, + revision: 0, + mounted: false, + pendingChange: null as Promise | null, + }); + const transition = (next: CellEditSessionState) => { + session.state = next; + setState(next); + }; + const isCurrent = (revision: number) => session.mounted && session.revision === revision; + + useEffect(() => { + session.mounted = true; + return () => { + session.mounted = false; + session.revision++; + session.pendingChange = null; + }; + }, [enabled, session]); + + const begin = (draft: TDraft) => { + if (session.state.status !== "idle") return; + session.revision++; + transition({ status: "editing", draft }); + }; + + const change = (draft: TDraft, callback?: () => unknown) => { + const revision = ++session.revision; + session.pendingChange = null; + transition({ status: "editing", draft }); + if (!callback) return; + if (!enabled) { + callback(); + return; + } + const reject = (error: unknown) => { + if (isCurrent(revision)) transition({ status: "failed", draft, error, source: "change" }); + }; + try { + const result = callback(); + if (isPromiseLike(result)) { + const pending = Promise.resolve(result).then( + () => { + if (isCurrent(revision) && session.pendingChange === pending) + session.pendingChange = null; + return true; + }, + (error: unknown) => { + if (isCurrent(revision) && session.pendingChange === pending) + session.pendingChange = null; + reject(error); + return false; + }, + ); + if (isCurrent(revision)) session.pendingChange = pending; + } + } catch (error) { + reject(error); + } + }; + + const discard = () => { + session.revision++; + session.pendingChange = null; + transition({ status: "idle" }); + }; + + const confirm = (callback: () => unknown, onSuccess?: () => void) => { + const current = session.state; + if ( + current.status === "idle" || + current.status === "confirming" || + (current.status === "failed" && current.source !== "commit") + ) + return; + if (!enabled) { + discard(); + onSuccess?.(); + callback(); + return; + } + const { draft } = current; + const revision = session.revision; + const pendingChange = session.pendingChange; + transition({ status: "confirming", draft }); + const isConfirming = () => isCurrent(revision) && session.state.status === "confirming"; + const complete = () => { + if (!isConfirming()) return; + transition({ status: "idle" }); + onSuccess?.(); + }; + const reject = (error: unknown) => { + if (isConfirming()) transition({ status: "failed", draft, error, source: "commit" }); + }; + const run = () => { + if (!isConfirming()) return; + try { + const result = callback(); + if (isPromiseLike(result)) { + void Promise.resolve(result).then(complete, reject); + return; + } + } catch (error) { + reject(error); + return; + } + complete(); + }; + if (pendingChange) { + void pendingChange.then((valid) => { + if (valid) run(); + }); + } else { + run(); + } + }; + + const fail = (error: unknown, source: "parse") => { + const current = session.state; + if (current.status === "idle") return; + session.revision++; + session.pendingChange = null; + transition({ status: "failed", draft: current.draft, error, source }); + }; + + const cancel = (callback?: () => unknown) => { + discard(); + if (!callback) return; + if (!enabled) { + callback(); + return; + } + // Restoration failures have no draft to attach to and must not recreate an orphan error. + try { + const result = callback(); + if (isPromiseLike(result)) void Promise.resolve(result).catch(() => {}); + } catch { + // The edit has already been cancelled. + } + }; + + return { state, getState: () => session.state, begin, change, confirm, fail, cancel }; +} From d03c88a492c585075eb6ebe5dc297722ab249f3a Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Thu, 22 Oct 2026 14:34:03 +0900 Subject: [PATCH 2/2] fix(data-table): remove built-in cell error reset action --- .changeset/quiet-cell-errors.md | 4 ++-- docs-manifest.json | 6 +++--- docs-src/components/data-table.docs.outline.md | 8 ++++---- docs/components/data-table.md | 8 ++++---- .../data-table-lab/editing-errors-demo.tsx | 12 ++++++------ .../data-table/data-table-errors.test.tsx | 17 ++++++++++------- .../src/components/data-table/editable-cell.tsx | 3 --- packages/core/src/components/data-table/i18n.ts | 2 -- .../core/src/components/data-table/types.ts | 3 ++- 9 files changed, 31 insertions(+), 32 deletions(-) diff --git a/.changeset/quiet-cell-errors.md b/.changeset/quiet-cell-errors.md index 70660ba69..e9b35a575 100644 --- a/.changeset/quiet-cell-errors.md +++ b/.changeset/quiet-cell-errors.md @@ -2,5 +2,5 @@ "@tailor-platform/app-shell": minor --- -Show cell-local error popovers by default when DataTable editing callbacks throw or return a rejected Promise. Keep pending and failed drafts editable, wait for a successful `onValueChange` check before confirmation, and clear drafts only after confirmation succeeds; Esc or Revert edit discards the draft and error together without trapping cell navigation. -Customize the popover body with `edit.error.render({ row, error })`, or set `edit.error: false` to retain eager edit completion without callback interception. Data adoption, persistence, and rollback remain consumer-owned. +Show cell-local error popovers by default when DataTable editing callbacks throw or return a rejected Promise. Keep pending and failed drafts editable, wait for a successful `onValueChange` check before confirmation, and clear drafts only after confirmation succeeds; Esc discards the draft and error together without trapping cell navigation. +The standard popover displays errors without a reset action. Customize its body and any consumer-owned actions with `edit.error.render({ row, error })`, or set `edit.error: false` to retain eager edit completion without callback interception. Data adoption, persistence, and rollback remain consumer-owned. diff --git a/docs-manifest.json b/docs-manifest.json index a6496cd99..9ea9b1c70 100644 --- a/docs-manifest.json +++ b/docs-manifest.json @@ -454,9 +454,9 @@ ], "hashes": { "typeSurface": "3ca8fce7d2efca2d", - "outline": "34a240cc8b38c38e", + "outline": "6fa3b40f1fe0b561", "snapshot": null, - "outputMd": "23265474c7780224", + "outputMd": "354b21e48e9605d0", "examples": "b3468077b9455521" } }, @@ -1751,7 +1751,7 @@ "packages/core/skills/app-shell-patterns/references/components/combobox.md": "dfb775c7c4307409", "packages/core/skills/app-shell-patterns/references/components/command-palette.md": "7e5bc675a593791a", "packages/core/skills/app-shell-patterns/references/components/csv-importer.md": "53d3c795ca21b9dc", - "packages/core/skills/app-shell-patterns/references/components/data-table.md": "23265474c7780224", + "packages/core/skills/app-shell-patterns/references/components/data-table.md": "354b21e48e9605d0", "packages/core/skills/app-shell-patterns/references/components/date-picker.md": "bc69f14829004f9d", "packages/core/skills/app-shell-patterns/references/components/default-header.md": "375e55f4b12fa2be", "packages/core/skills/app-shell-patterns/references/components/default-sidebar.md": "3422388a0cc78571", diff --git a/docs-src/components/data-table.docs.outline.md b/docs-src/components/data-table.docs.outline.md index 90e8b227d..ea04ffae7 100644 --- a/docs-src/components/data-table.docs.outline.md +++ b/docs-src/components/data-table.docs.outline.md @@ -449,7 +449,7 @@ Rows need a string or number `id`; rows without one remain read-only and produce - **Enter / Shift+Enter:** attempt confirmation in the same logical cell. Success ends editing and restores normal cell display; a pending callback keeps the draft visible, and failure keeps it editable with its error. Enter can retry a failed commit. A failed change check or numeric parse must be corrected before confirmation can proceed. Enter or clicking a focused display cell starts editing again. - **Tab / Shift+Tab:** attempt confirmation and move to the next / previous editable cell, skipping read-only cells. Callback failures and pending callbacks do not trap focus: their drafts remain in the original cell. At the ends, focus leaves the table normally. Malformed numeric input retains its existing Tab-blocking behavior. - **Esc:** cancel the edit and atomically discard its draft and error, returning to the latest `data`. Notify `onValueChange` only if the restored draft differs, without `onValueCommitted`; restoration failures cannot recreate an error after cancellation. Pending callback results are invalidated, not cancelled. If the error popover is open, the first Esc only closes those details; a subsequent Esc in the editor or error trigger cancels the edit. -- **Dropdowns:** type to filter choices. Exact labels rank first, then prefix matches, then substring matches, with original order retained within each rank. "None" is excluded while a search query is present and available with an empty query. Tab focuses a closed dropdown without opening it; one Enter opens it without confirming. While open, Enter attempts confirmation of the candidate in the same cell; Tab attempts confirmation of a selected or searched candidate and moves on. Tab without selecting or searching only moves focus. A keyboard-highlighted candidate takes precedence over ranked search matches; hovering does not count as an explicit selection. Clicking a choice also attempts confirmation. Outside click, blur, and Esc discard an ordinary unconfirmed search and restore the latest `data`, even for an exactly typed label or keyboard-highlighted candidate, without either callback. A selected candidate whose confirmation is pending or failed remains displayed even if the dropdown closes; Esc or Revert edit cancels it. Search/highlight changes emit neither callback; a different selection emits `onValueChange`, then invokes `onValueCommitted` only after the change check succeeds. +- **Dropdowns:** type to filter choices. Exact labels rank first, then prefix matches, then substring matches, with original order retained within each rank. "None" is excluded while a search query is present and available with an empty query. Tab focuses a closed dropdown without opening it; one Enter opens it without confirming. While open, Enter attempts confirmation of the candidate in the same cell; Tab attempts confirmation of a selected or searched candidate and moves on. Tab without selecting or searching only moves focus. A keyboard-highlighted candidate takes precedence over ranked search matches; hovering does not count as an explicit selection. Clicking a choice also attempts confirmation. Outside click, blur, and Esc discard an ordinary unconfirmed search and restore the latest `data`, even for an exactly typed label or keyboard-highlighted candidate, without either callback. A selected candidate whose confirmation is pending or failed remains displayed even if the dropdown closes; Esc cancels it. Search/highlight changes emit neither callback; a different selection emits `onValueChange`, then invokes `onValueCommitted` only after the change check succeeds. - **Dates:** selecting a day emits its draft and attempts confirmation immediately. Clear explicitly attempts confirmation of `null`. - **Date-times:** day and time are independent. Both must be valid to enable Done; an empty time does not become midnight. Done attempts confirmation of the timestamp in the AppShell timezone. Outside closing cancels an ordinary unfinished edit, but retains a pending or failed draft; Esc cancels either. Clear explicitly attempts confirmation of `null`. While a date editor has an active, pending, or failed draft, its trigger shows that raw draft date/time rather than the adopted value's formatter or custom `column.render`. - **Blur:** attempts confirmation of a changed direct-input value. It does not retry an already failed commit or duplicate an in-flight confirmation; failed and pending drafts survive leaving and re-entering the cell. Enter during IME composition confirms the conversion, not the cell. Composition changes may notify raw drafts; normalization waits until cell confirmation. @@ -473,7 +473,7 @@ Malformed numeric input is allowed while typing and can notify raw drafts. Enter By default, DataTable catches synchronous throws and rejections of Promises returned by `onValueChange` or `onValueCommitted`. The affected cell receives an error outline and an error icon. Click the icon, or press **Alt+ArrowDown** while focused in the cell, to open its popover. The error state is also exposed through `aria-invalid` and an associated description. Opening or using the error popover is not an ordinary editor blur: it does not confirm or discard the draft merely to inspect the error. -No extra configuration is needed for the standard UI. It displays a non-empty `Error.message`, otherwise the localized message “An error occurred”. It never serializes arbitrary thrown values or displays a stack trace. Throw user-facing messages; translate or sanitize internal errors before returning them to this UI. The popover includes a built-in **Revert edit** action (**編集を元に戻す** in Japanese), including when its body is customized, so the draft and error can be discarded together. +No extra configuration is needed for the standard UI. It displays a non-empty `Error.message`, otherwise the localized message “An error occurred”. It never serializes arbitrary thrown values or displays a stack trace. Throw user-facing messages; translate or sanitize internal errors before returning them to this UI. The standard popover displays the error only; it adds no reset or rollback action. Consumer-owned actions can be included in `error.render` when appropriate. ```tsx edit: { @@ -489,7 +489,7 @@ edit: { `saveReceived` is the screen's persistence function. DataTable keeps the raw draft while the returned Promise is pending. Successful completion clears that draft and displays the latest consumer-provided `data`; rejection retains the same draft with its error for correction, an explicit Enter retry, or cancellation. Only successful confirmation ends the edit. Repeated Enter or a subsequent blur does not invoke an in-flight confirmation again. This delays **edit completion**, not focus navigation, and does not add a save mode, automatic retry, optimistic row state, or automatic persistence. -To customize the body, provide `error.render`. Its `error` is `unknown`, because JavaScript can throw any value; `row` is the cell's current row. Popover placement, open/close behavior, the error indicator, and the built-in Revert edit action remain owned by DataTable. The body may include links or buttons. +To customize the body, provide `error.render`. Its `error` is `unknown`, because JavaScript can throw any value; `row` is the cell's current row. Popover placement, open/close behavior, and the error indicator remain owned by DataTable. The body may include consumer-owned links or buttons; DataTable adds no actions alongside it. ```tsx error: { @@ -515,7 +515,7 @@ edit: { With `false`, DataTable preserves eager edit completion: it clears the draft before invoking the confirmation callback, does not intercept synchronous exceptions, and does not observe returned Promises. Handle failures in the callback rather than relying on a hidden error state. Neither `error: true` nor `error: {}` is needed or accepted. Switching between `error: false` and handled mode remounts the editor, discarding its local session and invalidating old results. -The cell's editing session is either idle, editing a draft, confirming that draft, or retaining a failed draft with its error. **An error never exists without its target draft.** New input clears the previous failure and invalidates older asynchronous results. A failed `onValueChange` prevents `onValueCommitted` from running; a pending change check must succeed first. A failed commit can be retried without losing its draft. Esc and Revert edit atomically discard the draft and error, restore the latest data, and invalidate pending results. Unmount discards the whole session; it is not persisted across page changes. Existing malformed-number feedback keeps its existing Enter/Tab and blur behavior. +The cell's editing session is either idle, editing a draft, confirming that draft, or retaining a failed draft with its error. **An error never exists without its target draft.** New input clears the previous failure and invalidates older asynchronous results. A failed `onValueChange` prevents `onValueCommitted` from running; a pending change check must succeed first. A failed commit can be retried without losing its draft. Esc atomically discards the draft and error, restores the latest data, and invalidates pending results. Unmount discards the whole session; it is not persisted across page changes. Existing malformed-number feedback keeps its existing Enter/Tab and blur behavior. Only the returned Promise is observable. A detached operation such as `void saveReceived(...)` is outside this lifecycle; return or await the operation from the callback. The callback types remain `=> void` for compatibility with existing handlers, including async functions; successful return values are ignored. A stale completion cannot clear a newer draft, replace its error, or finish its editor. This protection does not cancel requests, undo callback side effects, roll back optimistic writes, or prevent the consumer from writing stale data, so keep the screen's own request revision or conflict handling when needed. diff --git a/docs/components/data-table.md b/docs/components/data-table.md index 1ba671a5f..4bda53f66 100644 --- a/docs/components/data-table.md +++ b/docs/components/data-table.md @@ -443,7 +443,7 @@ Rows need a string or number `id`; rows without one remain read-only and produce - **Enter / Shift+Enter:** attempt confirmation in the same logical cell. Success ends editing and restores normal cell display; a pending callback keeps the draft visible, and failure keeps it editable with its error. Enter can retry a failed commit. A failed change check or numeric parse must be corrected before confirmation can proceed. Enter or clicking a focused display cell starts editing again. - **Tab / Shift+Tab:** attempt confirmation and move to the next / previous editable cell, skipping read-only cells. Callback failures and pending callbacks do not trap focus: their drafts remain in the original cell. At the ends, focus leaves the table normally. Malformed numeric input retains its existing Tab-blocking behavior. - **Esc:** cancel the edit and atomically discard its draft and error, returning to the latest `data`. Notify `onValueChange` only if the restored draft differs, without `onValueCommitted`; restoration failures cannot recreate an error after cancellation. Pending callback results are invalidated, not cancelled. If the error popover is open, the first Esc only closes those details; a subsequent Esc in the editor or error trigger cancels the edit. -- **Dropdowns:** type to filter choices. Exact labels rank first, then prefix matches, then substring matches, with original order retained within each rank. "None" is excluded while a search query is present and available with an empty query. Tab focuses a closed dropdown without opening it; one Enter opens it without confirming. While open, Enter attempts confirmation of the candidate in the same cell; Tab attempts confirmation of a selected or searched candidate and moves on. Tab without selecting or searching only moves focus. A keyboard-highlighted candidate takes precedence over ranked search matches; hovering does not count as an explicit selection. Clicking a choice also attempts confirmation. Outside click, blur, and Esc discard an ordinary unconfirmed search and restore the latest `data`, even for an exactly typed label or keyboard-highlighted candidate, without either callback. A selected candidate whose confirmation is pending or failed remains displayed even if the dropdown closes; Esc or Revert edit cancels it. Search/highlight changes emit neither callback; a different selection emits `onValueChange`, then invokes `onValueCommitted` only after the change check succeeds. +- **Dropdowns:** type to filter choices. Exact labels rank first, then prefix matches, then substring matches, with original order retained within each rank. "None" is excluded while a search query is present and available with an empty query. Tab focuses a closed dropdown without opening it; one Enter opens it without confirming. While open, Enter attempts confirmation of the candidate in the same cell; Tab attempts confirmation of a selected or searched candidate and moves on. Tab without selecting or searching only moves focus. A keyboard-highlighted candidate takes precedence over ranked search matches; hovering does not count as an explicit selection. Clicking a choice also attempts confirmation. Outside click, blur, and Esc discard an ordinary unconfirmed search and restore the latest `data`, even for an exactly typed label or keyboard-highlighted candidate, without either callback. A selected candidate whose confirmation is pending or failed remains displayed even if the dropdown closes; Esc cancels it. Search/highlight changes emit neither callback; a different selection emits `onValueChange`, then invokes `onValueCommitted` only after the change check succeeds. - **Dates:** selecting a day emits its draft and attempts confirmation immediately. Clear explicitly attempts confirmation of `null`. - **Date-times:** day and time are independent. Both must be valid to enable Done; an empty time does not become midnight. Done attempts confirmation of the timestamp in the AppShell timezone. Outside closing cancels an ordinary unfinished edit, but retains a pending or failed draft; Esc cancels either. Clear explicitly attempts confirmation of `null`. While a date editor has an active, pending, or failed draft, its trigger shows that raw draft date/time rather than the adopted value's formatter or custom `column.render`. - **Blur:** attempts confirmation of a changed direct-input value. It does not retry an already failed commit or duplicate an in-flight confirmation; failed and pending drafts survive leaving and re-entering the cell. Enter during IME composition confirms the conversion, not the cell. Composition changes may notify raw drafts; normalization waits until cell confirmation. @@ -467,7 +467,7 @@ Malformed numeric input is allowed while typing and can notify raw drafts. Enter By default, DataTable catches synchronous throws and rejections of Promises returned by `onValueChange` or `onValueCommitted`. The affected cell receives an error outline and an error icon. Click the icon, or press **Alt+ArrowDown** while focused in the cell, to open its popover. The error state is also exposed through `aria-invalid` and an associated description. Opening or using the error popover is not an ordinary editor blur: it does not confirm or discard the draft merely to inspect the error. -No extra configuration is needed for the standard UI. It displays a non-empty `Error.message`, otherwise the localized message “An error occurred”. It never serializes arbitrary thrown values or displays a stack trace. Throw user-facing messages; translate or sanitize internal errors before returning them to this UI. The popover includes a built-in **Revert edit** action (**編集を元に戻す** in Japanese), including when its body is customized, so the draft and error can be discarded together. +No extra configuration is needed for the standard UI. It displays a non-empty `Error.message`, otherwise the localized message “An error occurred”. It never serializes arbitrary thrown values or displays a stack trace. Throw user-facing messages; translate or sanitize internal errors before returning them to this UI. The standard popover displays the error only; it adds no reset or rollback action. Consumer-owned actions can be included in `error.render` when appropriate. ```tsx edit: { @@ -483,7 +483,7 @@ edit: { `saveReceived` is the screen's persistence function. DataTable keeps the raw draft while the returned Promise is pending. Successful completion clears that draft and displays the latest consumer-provided `data`; rejection retains the same draft with its error for correction, an explicit Enter retry, or cancellation. Only successful confirmation ends the edit. Repeated Enter or a subsequent blur does not invoke an in-flight confirmation again. This delays **edit completion**, not focus navigation, and does not add a save mode, automatic retry, optimistic row state, or automatic persistence. -To customize the body, provide `error.render`. Its `error` is `unknown`, because JavaScript can throw any value; `row` is the cell's current row. Popover placement, open/close behavior, the error indicator, and the built-in Revert edit action remain owned by DataTable. The body may include links or buttons. +To customize the body, provide `error.render`. Its `error` is `unknown`, because JavaScript can throw any value; `row` is the cell's current row. Popover placement, open/close behavior, and the error indicator remain owned by DataTable. The body may include consumer-owned links or buttons; DataTable adds no actions alongside it. ```tsx error: { @@ -509,7 +509,7 @@ edit: { With `false`, DataTable preserves eager edit completion: it clears the draft before invoking the confirmation callback, does not intercept synchronous exceptions, and does not observe returned Promises. Handle failures in the callback rather than relying on a hidden error state. Neither `error: true` nor `error: {}` is needed or accepted. Switching between `error: false` and handled mode remounts the editor, discarding its local session and invalidating old results. -The cell's editing session is either idle, editing a draft, confirming that draft, or retaining a failed draft with its error. **An error never exists without its target draft.** New input clears the previous failure and invalidates older asynchronous results. A failed `onValueChange` prevents `onValueCommitted` from running; a pending change check must succeed first. A failed commit can be retried without losing its draft. Esc and Revert edit atomically discard the draft and error, restore the latest data, and invalidate pending results. Unmount discards the whole session; it is not persisted across page changes. Existing malformed-number feedback keeps its existing Enter/Tab and blur behavior. +The cell's editing session is either idle, editing a draft, confirming that draft, or retaining a failed draft with its error. **An error never exists without its target draft.** New input clears the previous failure and invalidates older asynchronous results. A failed `onValueChange` prevents `onValueCommitted` from running; a pending change check must succeed first. A failed commit can be retried without losing its draft. Esc atomically discards the draft and error, restores the latest data, and invalidates pending results. Unmount discards the whole session; it is not persisted across page changes. Existing malformed-number feedback keeps its existing Enter/Tab and blur behavior. Only the returned Promise is observable. A detached operation such as `void saveReceived(...)` is outside this lifecycle; return or await the operation from the callback. The callback types remain `=> void` for compatibility with existing handlers, including async functions; successful return values are ignored. A stale completion cannot clear a newer draft, replace its error, or finish its editor. This protection does not cancel requests, undo callback side effects, roll back optimistic writes, or prevent the consumer from writing stale data, so keep the screen's own request revision or conflict handling when needed. diff --git a/examples/vite-app/src/pages/showcase/data-table-lab/editing-errors-demo.tsx b/examples/vite-app/src/pages/showcase/data-table-lab/editing-errors-demo.tsx index 0498e6914..78e999ec1 100644 --- a/examples/vite-app/src/pages/showcase/data-table-lab/editing-errors-demo.tsx +++ b/examples/vite-app/src/pages/showcase/data-table-lab/editing-errors-demo.tsx @@ -107,15 +107,15 @@ export function EditingErrorsDemo() {
  • In Draft note, type ! to throw from{" "} onValueChange. The failed draft stays visible and cannot be confirmed until - you remove !. Press Esc or use Revert edit to restore the - adopted note and clear its error together. + you remove !. Press Esc to restore the adopted note and clear its error + together.
  • Set Async quantity to 13 and press Enter. The returned Promise rejects after 600 ms; 13 stays visible while pending and after failure. Enter retries the same draft. Change it to 14 to save successfully, - or press Esc / use Revert edit to restore the adopted quantity. To test - stale failures, confirm 13 and start typing another value before the rejection arrives. + or press Esc to restore the adopted quantity. To test stale failures, confirm 13 and start + typing another value before the rejection arrives.
  • Set Custom code to bad and press Enter. Open its error @@ -128,8 +128,8 @@ export function EditingErrorsDemo() { Click an error icon or press Alt+ArrowDown in its editor to inspect the popover without confirming or discarding the draft. Pending and failed drafts survive Tab or blur without trapping focus. Only successful confirmation clears the draft; only the consumer adopts - successful values. Closing error details keeps the draft; Esc in the editor or Revert edit - cancels the draft and its error together. + successful values. The standard popover adds no reset action. Closing error details keeps + the draft; Esc in the editor cancels the draft and its error together. diff --git a/packages/core/src/components/data-table/data-table-errors.test.tsx b/packages/core/src/components/data-table/data-table-errors.test.tsx index 68ee83969..b44b8c9c3 100644 --- a/packages/core/src/components/data-table/data-table-errors.test.tsx +++ b/packages/core/src/components/data-table/data-table-errors.test.tsx @@ -128,6 +128,7 @@ describe("DataTable callback error presentation", () => { await user.click(errorTrigger()); const popup = await screen.findByRole("dialog", { name: "Error in Value" }); expect(within(popup).getByText(failure.message)).toBeDefined(); + expect(within(popup).queryByRole("button")).toBeNull(); expect(fail).toHaveBeenCalledExactlyOnceWith(ROWS[0], "Changed"); expect(input.value).toBe("Changed"); expect(input.readOnly).toBe(false); @@ -192,6 +193,7 @@ describe("DataTable callback error presentation", () => { expect(within(popup).getByText("1: INVALID")).toBeDefined(); expect(renderError).toHaveBeenCalledWith({ row: ROWS[0], error: failure }); const action = within(popup).getByRole("button", { name: "Explain this error" }); + expect(within(popup).getAllByRole("button")).toEqual([action]); await vi.waitFor(() => expect(document.activeElement).toBe(action)); await user.keyboard("{Enter}"); expect(onAction).toHaveBeenCalledTimes(1); @@ -379,21 +381,19 @@ describe("DataTable callback error presentation", () => { expect(queryErrorTrigger()).toBeNull(); }); - it("reverts a failed draft and error together to the latest consumer data", async () => { + it("uses Escape to revert a failed draft and error together to the latest consumer data", () => { const onValueChange = vi.fn(); const onValueCommitted = vi.fn(() => { throw new Error("Not accepted"); }); - const { input, user, updateRows } = renderEditor({ onValueChange, onValueCommitted }); + const { input, updateRows } = renderEditor({ onValueChange, onValueCommitted }); change(input); fireEvent.keyDown(input, { key: "Enter" }); const latest = { id: "1", value: "Updated externally" }; updateRows([latest]); expect(input.value).toBe("Changed"); expect(input.getAttribute("aria-invalid")).toBe("true"); - await user.click(errorTrigger()); - const popup = await screen.findByRole("dialog", { name: "Error in Value" }); - await user.click(within(popup).getByRole("button", { name: "Revert edit" })); + fireEvent.keyDown(input, { key: "Escape" }); expect(input.value).toBe("Updated externally"); expect(input.readOnly).toBe(true); expect(input.getAttribute("aria-invalid")).toBeNull(); @@ -571,8 +571,11 @@ describe("DataTable callback error presentation", () => { expect(trigger.closest("td")?.textContent).toContain("2026-10-20"); expect(trigger.getAttribute("aria-invalid")).toBe("true"); await user.click(errorTrigger()); - const popup = await screen.findByRole("dialog", { name: "Error in Value" }); - await user.click(within(popup).getByRole("button", { name: "Revert edit" })); + await screen.findByRole("dialog", { name: "Error in Value" }); + await user.keyboard("{Escape}"); + await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + expect(trigger.getAttribute("aria-label")).toBe("Value, 2026-10-20"); + fireEvent.keyDown(trigger, { key: "Escape" }); expect(trigger.getAttribute("aria-label")).toBe(initialLabel); expect(trigger.getAttribute("aria-invalid")).toBeNull(); expect(queryErrorTrigger()).toBeNull(); diff --git a/packages/core/src/components/data-table/editable-cell.tsx b/packages/core/src/components/data-table/editable-cell.tsx index 5784d9521..0c5d9b5a2 100644 --- a/packages/core/src/components/data-table/editable-cell.tsx +++ b/packages/core/src/components/data-table/editable-cell.tsx @@ -369,9 +369,6 @@ function EditableCellFrame({ }} > {callbackError.content} - diff --git a/packages/core/src/components/data-table/i18n.ts b/packages/core/src/components/data-table/i18n.ts index f52c5a12f..6425f089b 100644 --- a/packages/core/src/components/data-table/i18n.ts +++ b/packages/core/src/components/data-table/i18n.ts @@ -135,7 +135,6 @@ export const dataTableLabels = defineI18nLabels({ // Inline cell editing — numeric parsing feedback editNotANumber: "Enter a number", editRevertHint: "Press Esc to undo", - editRevert: "Revert edit", editNone: "None", editNoMatches: "No matches", editClear: "Clear", @@ -262,7 +261,6 @@ export const dataTableLabels = defineI18nLabels({ // Inline cell editing editNotANumber: "数値を入力してください", editRevertHint: "Escキーで元に戻せます", - editRevert: "編集を元に戻す", editNone: "なし", editNoMatches: "一致する項目がありません", editClear: "クリア", diff --git a/packages/core/src/components/data-table/types.ts b/packages/core/src/components/data-table/types.ts index 9e9090628..b5d4b4876 100644 --- a/packages/core/src/components/data-table/types.ts +++ b/packages/core/src/components/data-table/types.ts @@ -131,7 +131,8 @@ interface CellEditBase, TValue, TDraft = TV * Callback failures use a standard error popover by default. Pass `false` to * disable interception and retain eager edit completion, or `render` to replace * the popover's body. Failed drafts stay visible until corrected or cancelled; - * Esc / Revert edit restores current data and clears the error together. + * Esc restores current data and clears the error together. The standard + * popover adds no reset action; custom content may include consumer-owned actions. */ error?: | false