From aec8ec5f7935be29d37bb94150153d3e9d6388a4 Mon Sep 17 00:00:00 2001 From: IzumiSy Date: Thu, 22 Oct 2026 13:36:15 +0900 Subject: [PATCH 1/8] 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/8] 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 From ddaf5421f6d4931fe858bdff58a2a06b533188b7 Mon Sep 17 00:00:00 2001 From: pradeep <6978383+itsprade@users.noreply.github.com> Date: Mon, 26 Oct 2026 09:53:03 +0530 Subject: [PATCH 3/8] feat(data-table): refine the cell-error popover UI (#638) * feat(data-table): give the default cell-error popover an error look and undo hint The standard popover body showed only the message, so it read like a neutral tooltip. It now leads with a destructive alert icon, shows the message in medium weight, and adds the existing "Press Esc to undo" hint. Custom error.render content is unchanged. Co-Authored-By: Claude Opus 5.5 * feat(data-table): tighten the cell-error popover Drop the undo hint and the popup border, reduce padding and cap the width at max-w-xs so long messages wrap. The lab's Draft note column now throws a longer message on "?" to show wrapping. Co-Authored-By: Claude Opus 5.5 * feat(data-table): refine cell-error popover placement and tint - 1px error ring on the cell, including while focused - popup opens directly under the cell, aligned to its left edge - light red tint (Alert's danger-surface) layered over the opaque popover colour so table lines don't show through; softer shadow-sm - icon centred on the first text line, at 70% of the text colour - lab: "!!" in Draft note throws a long message to show wrapping Co-Authored-By: Claude Opus 5.5 * feat(data-table): 1.5px error outline, heavier error icons - cell error ring is 1.5px (resting and focused) - error icons in the cell and popover use strokeWidth 2.5 - drop the focus ring on the in-cell error trigger; it drew a second box around the icon when focus returned to it on popover close Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- .../data-table-lab/editing-errors-demo.tsx | 8 +++- .../components/data-table/editable-cell.tsx | 47 ++++++++++++++++--- 2 files changed, 47 insertions(+), 8 deletions(-) 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 78e999ec1..0a9d287a8 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 @@ -18,6 +18,10 @@ type ErrorDemoRow = { const { column } = createColumnHelper(); function checkNote(value: string | null) { + if (value?.includes("!!")) + throw new Error( + "Notes can't contain repeated exclamation marks. Keep notes factual and send anything urgent to the receiving team through the comments panel so it is tracked against this purchase order.", + ); if (value?.includes("!")) throw new Error("Remove ! from the note"); } @@ -107,8 +111,8 @@ 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 to restore the adopted note and clear its error - together. + you remove !. Type !! to see a longer message wrap. Press Esc to + restore the adopted note and clear its error together.
  • Set Async quantity to 13 and press Enter. The returned diff --git a/packages/core/src/components/data-table/editable-cell.tsx b/packages/core/src/components/data-table/editable-cell.tsx index 8cacabb80..c4f447359 100644 --- a/packages/core/src/components/data-table/editable-cell.tsx +++ b/packages/core/src/components/data-table/editable-cell.tsx @@ -128,7 +128,7 @@ const CELL_FOCUS_CLASS_NAME = cn( // 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"; + "astw:ring-[1.5px] astw:ring-inset astw:ring-destructive astw:focus:ring-[1.5px] astw:focus:ring-destructive astw:focus-visible:ring-[1.5px] astw:focus-visible:ring-destructive"; // An editable cell says so at rest with a subtle tint instead of an icon. It's // a background *image* layered over whatever colour the cell already has (the @@ -181,6 +181,24 @@ interface CellErrorUI { containsTarget: (target: EventTarget | null) => boolean; } +// The standard popover body: the alert icon makes it read as an error rather +// than a tooltip. +function DefaultCellErrorContent({ message }: { message: string }) { + return ( +
    + {/* One line-height tall, so the icon centres on the first line of text. */} + + +

    {message}

    +
    + ); +} + function useCellValue>( row: TRow, col: EditableColumn, @@ -219,7 +237,11 @@ function useCellValue>( failure, id: errorId, message: renderError ? t("editCallbackError") : message, - content: renderError ? renderError({ row, error: failure.error }) : message, + content: renderError ? ( + renderError({ row, error: failure.error }) + ) : ( + + ), triggerRef, popupRef, containsTarget: containsErrorTarget, @@ -351,18 +373,31 @@ function EditableCellFrame({ aria-label={t("editShowError", { label })} aria-keyshortcuts="Alt+ArrowDown" tabIndex={-1} - className="astw:absolute astw:inset-y-0 astw:right-2 astw:z-10 astw:flex astw:items-center astw:text-destructive astw:outline-none astw:focus-visible:ring-2 astw:focus-visible:ring-destructive astw:[td:last-child>&]:right-6" + className="astw:absolute astw:inset-y-0 astw:right-2 astw:z-10 astw:flex astw:items-center astw:text-destructive astw:outline-none astw:[td:last-child>&]:right-6" onContextMenu={(event) => event.stopPropagation()} > -
  • 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 to restore the adopted quantity. To test stale failures, confirm 13 and start - typing another value before the rejection arrives. + or close its error details and 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 - popover and use Show valid code example to try interactive custom - content. The failed bad draft remains editable; change it to{" "} - SKU-101 and confirm to adopt a valid code. + Set Custom code to bad and press Enter. Its error popover + opens automatically; press Alt+ArrowDown to focus the details and use{" "} + Show valid code example to try interactive custom content. The failed{" "} + bad draft remains editable; change it to SKU-101 and confirm to + adopt a valid code.
    • diff --git a/examples/vite-app/src/pages/showcase/data-table-lab/inline-editing-demo.tsx b/examples/vite-app/src/pages/showcase/data-table-lab/inline-editing-demo.tsx index 7186b6acf..e6f46d5d5 100644 --- a/examples/vite-app/src/pages/showcase/data-table-lab/inline-editing-demo.tsx +++ b/examples/vite-app/src/pages/showcase/data-table-lab/inline-editing-demo.tsx @@ -1,4 +1,10 @@ -import { DataTable, createColumnHelper, useDataTable, useToast } from "@tailor-platform/app-shell"; +import { + CellEditError, + DataTable, + createColumnHelper, + useDataTable, + useToast, +} from "@tailor-platform/app-shell"; import { useRef, useState } from "react"; // ─── Dummy data ────────────────────────────────────────────────────────────── @@ -218,8 +224,7 @@ export function InlineEditingDemo() { canEdit: (row) => row.status === "pending", onValueCommitted: (row, value) => { if (value === null) { - reject(`${row.id}: Supplier is required`); - return; + throw new CellEditError(`${row.id}: Supplier is required`); } update(row.id, { supplierId: value }, `Supplier → ${value}`); }, @@ -248,8 +253,9 @@ export function InlineEditingDemo() { canEdit: (_row, { selected }) => selected, onValueCommitted: (row, value) => { if (value !== null && (!Number.isInteger(value) || value < 0 || value > row.ordered)) { - reject(`${row.id}: Received must be a whole number from 0 to ${row.ordered}`); - return; + throw new CellEditError( + `${row.id}: Received must be a whole number from 0 to ${row.ordered}`, + ); } update(row.id, { received: value }, `Received → ${value ?? "empty"}`); }, @@ -265,10 +271,9 @@ export function InlineEditingDemo() { onValueCommitted: (row, value) => { const scale = row.currency === "JPY" ? 1 : 100; if (value === null || value < 0 || Math.round(value * scale) / scale !== value) { - reject( + throw new CellEditError( `${row.id}: Unit price is required, non-negative, and must fit ${row.currency} precision`, ); - return; } const request = priceSaves.current.get(row.id)!; const revision = ++request.revision; @@ -305,8 +310,7 @@ export function InlineEditingDemo() { edit: { onValueCommitted: (row, value) => { if (value === null || value < 0 || value > 100 || Math.round(value * 10) / 10 !== value) { - reject(`${row.id}: Discount must be 0–100 with at most one decimal`); - return; + throw new CellEditError(`${row.id}: Discount must be 0–100 with at most one decimal`); } update(row.id, { discount: value }, `Discount → ${value}%`); }, @@ -332,8 +336,9 @@ export function InlineEditingDemo() { value !== null && (value < 0 || value > 500 || Math.round(value * 1000) / 1000 !== value) ) { - reject(`${row.id}: Weight must be 0–500 kg with at most three decimals`); - return; + throw new CellEditError( + `${row.id}: Weight must be 0–500 kg with at most three decimals`, + ); } update(row.id, { weightKg: value }, `Weight → ${value ?? "empty"} kg`); }, @@ -347,8 +352,7 @@ export function InlineEditingDemo() { edit: { onValueCommitted: (row, value) => { if (value === null || value.trim() === "") { - reject(`${row.id}: Lot no. is required`); - return; + throw new CellEditError(`${row.id}: Lot no. is required`); } update(row.id, { lot: value }, `Lot → ${value}`); }, @@ -362,8 +366,7 @@ export function InlineEditingDemo() { edit: { onValueCommitted: (row, value) => { if (value !== null && value < "2026-09-01") { - reject(`${row.id}: Expected must be September 2026 or later`); - return; + throw new CellEditError(`${row.id}: Expected must be September 2026 or later`); } update(row.id, { expected: value }, `Expected → ${value ?? "cleared"}`); }, @@ -390,8 +393,7 @@ export function InlineEditingDemo() { edit: { onValueCommitted: (row, value) => { if (value !== null && (value < "2026-10-01" || value > "2027-12-31")) { - reject(`${row.id}: Best before must be October 2026–December 2027`); - return; + throw new CellEditError(`${row.id}: Best before must be October 2026–December 2027`); } update(row.id, { bestBefore: value }, `Best before → ${value ?? "cleared"}`); }, @@ -420,8 +422,7 @@ export function InlineEditingDemo() { edit: { onValueCommitted: (row, value) => { if (value === null) { - reject(`${row.id}: Status is required`); - return; + throw new CellEditError(`${row.id}: Status is required`); } update(row.id, { status: value as LineStatus }, `Status → ${value}`); }, @@ -466,9 +467,10 @@ export function InlineEditingDemo() {

    Consumer-owned rules and saving. Editors notify on Enter, Tab, blur, or a - pick. This screen accepts or rejects the value: rejected business values return to the - existing data, without adding cell errors or blocking Tab navigation. Saving has no built-in - indicator, rollback, or leave-page prompt. + pick. Invalid business values throw CellEditError, retaining the draft with a + cell error without trapping Tab navigation. This screen owns saving indicators and + optimistic rollback; saving failures still use its own feedback. There is no leave-page + prompt.

    {feedback && (

    { A goods receipt using onValueCommitted — scroll right for every editor. Editable cells are tinted; hover a dropdown or date cell to see its editor icon, then click anywhere in a cell. Enter confirms without moving focus, Tab moves to the next - editable cell, and Esc undoes a draft.{" "} - The screen owns validation and saving: a rejected business value - returns to the current data with a message outside the table; it does not block Tab - navigation or create a cell error. Tick a row to edit Received (whole - numbers from 0 to ordered), try 150 in Discount % (0–100, one - decimal), or more than 500 in Weight (kg) (three decimals).{" "} - Unit price updates locally before saving; values above 10,000 fail - after a delay and are rolled back by this screen. Supplier and Status reject "None"; + editable cell, and Esc closes open error details first, then undoes a draft.{" "} + The screen owns validation and saving: invalid business values throw{" "} + CellEditError, retaining the draft with a cell error without trapping Tab + navigation. Tick a row to edit Received (whole numbers from 0 to + ordered), try 150 in Discount % (0–100, one decimal), or more than + 500 in Weight (kg) (three decimals). Unit price{" "} + updates locally before saving; values above 10,000 fail after a delay, are rolled back + by this screen, and show consumer-owned feedback. Supplier and Status reject "None"; Lot no. rejects empty text. Warehouse, QC, and Note can be cleared. Dates use an unconstrained calendar: Expected rejects days before September 2026, Best before accepts October 2026–December 2027, and Received at adds a time field. There is no diff --git a/packages/core/src/components/data-table/cell-edit-error.ts b/packages/core/src/components/data-table/cell-edit-error.ts new file mode 100644 index 000000000..780f403ed --- /dev/null +++ b/packages/core/src/components/data-table/cell-edit-error.ts @@ -0,0 +1,7 @@ +/** + * A cell-editing failure whose message is safe to display to the user. + * Translate the message before throwing; other errors use generic feedback. + */ +export class CellEditError extends globalThis.Error { + override name = "CellEditError"; +} 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 b44b8c9c3..bc5f57913 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 @@ -3,7 +3,7 @@ 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 { CellEditError, type TextCellEditOptions } from "@/index"; import { createAppShellWrapper } from "../../../tests/test-utils"; import { DataTable } from "./data-table"; import { useDataTable } from "./use-data-table"; @@ -105,15 +105,22 @@ const errorTrigger = () => screen.getByRole("button", { name: "Show error for Va const queryErrorTrigger = () => screen.queryByRole("button", { name: "Show error for Value" }); describe("DataTable callback error presentation", () => { + it("exports CellEditError as a named native error", () => { + const error = new CellEditError("Enter a quantity"); + expect(error).toBeInstanceOf(Error); + expect(error.name).toBe("CellEditError"); + expect(error.message).toBe("Enter a quantity"); + }); + it.each(["change", "commit"] as const)( - "shows a synchronous %s failure in the default popover without opting in", + "automatically shows a synchronous %s failure without moving focus", async (source) => { - const failure = new Error("Cannot accept this value"); + const failure = new CellEditError("Cannot accept this value"); const fail = vi.fn(() => { throw failure; }); const onValueCommitted = source === "commit" ? fail : vi.fn(); - const { input, user } = renderEditor({ + const { input } = renderEditor({ onValueChange: source === "change" ? fail : undefined, onValueCommitted, }); @@ -124,11 +131,10 @@ describe("DataTable callback error presentation", () => { 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(within(popup).queryByRole("button")).toBeNull(); + expect(within(popup).getByRole("button", { name: "Close error details" })).toBeDefined(); + expect(document.activeElement).toBe(input); expect(fail).toHaveBeenCalledExactlyOnceWith(ROWS[0], "Changed"); expect(input.value).toBe("Changed"); expect(input.readOnly).toBe(false); @@ -143,7 +149,7 @@ describe("DataTable callback error presentation", () => { "retains the draft while a returned %s promise is pending and after rejection", async (source) => { const request = deferred(); - const { input, user } = renderEditor({ + const { input } = renderEditor({ onValueChange: source === "change" ? () => request.promise : undefined, onValueCommitted: source === "commit" ? () => request.promise : noop, }); @@ -155,16 +161,92 @@ describe("DataTable callback error presentation", () => { expect(input.readOnly).toBe(false); expect(input.value).toBe("Changed"); expect(queryErrorTrigger()).toBeNull(); - await rejectRequest(request, new Error("Server rejected the value")); + await rejectRequest(request, new CellEditError("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(); + expect(document.activeElement).toBe(input); + }, + ); + + it("dismisses only the details until a new failure and supports mouse and keyboard reopening", async () => { + const onValueChange = vi.fn(() => { + throw new Error("Draft rejected"); + }); + const onValueCommitted = vi.fn(); + const { input, user, updateRows } = renderEditor({ onValueChange, onValueCommitted }); + change(input); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + await user.click(within(popup).getByRole("button", { name: "Close error details" })); + await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + expect(queryErrorTrigger()).not.toBeNull(); + updateRows([{ ...ROWS[0] }]); + await user.click(screen.getByRole("button", { name: "Outside" })); + await user.click(input); + expect(screen.queryByRole("dialog")).toBeNull(); + expect(onValueChange).toHaveBeenCalledTimes(1); + expect(onValueCommitted).not.toHaveBeenCalled(); + + await user.click(errorTrigger()); + const reopened = await screen.findByRole("dialog", { name: "Error in Value" }); + await vi.waitFor(() => + expect(document.activeElement).toBe( + within(reopened).getByRole("button", { name: "Close error details" }), + ), + ); + await user.keyboard("{Escape}"); + await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + await user.keyboard("{Alt>}{ArrowDown}{/Alt}"); + const keyboardPopup = await screen.findByRole("dialog", { name: "Error in Value" }); + const close = within(keyboardPopup).getByRole("button", { name: "Close error details" }); + await vi.waitFor(() => expect(document.activeElement).toBe(close)); + await user.keyboard("{Enter}"); + await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + + await user.click(input); + fireEvent.change(input, { target: { value: "New draft" } }); + expect(await screen.findByRole("dialog", { name: "Error in Value" })).toBeDefined(); + expect(document.activeElement).toBe(input); + expect(input.value).toBe("New draft"); + expect(onValueChange).toHaveBeenCalledTimes(2); + expect(onValueCommitted).not.toHaveBeenCalled(); + }); + + it.each(["outside", "next cell"] as const)( + "auto-opens an async failure after moving to %s without stealing focus on open or dismissal", + async (destination) => { + const save = deferred(); + const onValueCommitted = vi.fn(() => save.promise); + const { input, inputs, user } = renderEditor( + { onValueCommitted }, + { rows: [...ROWS, { id: "2", value: "Second" }] }, + ); + const target = + destination === "outside" ? screen.getByRole("button", { name: "Outside" }) : inputs[1]; + change(input); + if (destination === "outside") await user.click(target); + else await user.tab(); + expect(document.activeElement).toBe(target); + await rejectRequest(save, new CellEditError("Save failed")); + expect( + within(await screen.findByRole("dialog", { name: "Error in Value" })).getByText( + "Save failed", + ), + ).toBeDefined(); + expect(document.activeElement).toBe(target); + await user.click(target); + await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + expect(document.activeElement).toBe(target); + expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + expect(onValueCommitted).toHaveBeenCalledTimes(1); }, ); @@ -188,12 +270,18 @@ describe("DataTable callback error presentation", () => { error: { render: renderError }, }); change(input); - fireEvent.keyDown(input, { key: "ArrowDown", altKey: true }); const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + expect(document.activeElement).toBe(input); + await user.keyboard("{Alt>}{ArrowDown}{/Alt}"); + expect(document.activeElement).toBe(popup); + await user.tab(); 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]); + expect(within(popup).getAllByRole("button")).toEqual([ + action, + within(popup).getByRole("button", { name: "Close error details" }), + ]); await vi.waitFor(() => expect(document.activeElement).toBe(action)); await user.keyboard("{Enter}"); expect(onAction).toHaveBeenCalledTimes(1); @@ -213,39 +301,93 @@ describe("DataTable callback error presentation", () => { 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, + it.each([ + new Error("Internal server details"), + new TypeError("Internal type details"), + Object.assign(new Error("Not a CellEditError instance"), { name: "CellEditError" }), + new CellEditError(""), + null, + undefined, + false, + "not an Error", + ])("uses generic visual and accessible feedback for thrown %j", async (failure) => { + const { input } = renderEditor({ + onValueChange: () => { + throw failure; + }, + onValueCommitted: noop, + }); + change(input); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + expect(within(popup).getByText("An error occurred")).toBeDefined(); + expect(document.getElementById(input.getAttribute("aria-describedby")!)?.textContent).toBe( + "An error occurred. Press Alt+ArrowDown for error details", + ); + expect(popup.textContent).toBe("An error occurred"); + expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + }); + + it.each(["change", "commit"] as const)( + "hides unexpected errors from rejected %s promises", + async (source) => { + const request = deferred(); + const { input } = renderEditor({ + onValueChange: source === "change" ? () => request.promise : undefined, + onValueCommitted: source === "commit" ? () => request.promise : noop, }); change(input); - await user.click(errorTrigger()); + if (source === "commit") fireEvent.keyDown(input, { key: "Enter" }); + await rejectRequest(request, new Error("Internal server details")); const popup = await screen.findByRole("dialog", { name: "Error in Value" }); - expect(within(popup).getByText("An error occurred")).toBeDefined(); + expect(popup.textContent).toBe("An error occurred"); + expect(document.getElementById(input.getAttribute("aria-describedby")!)?.textContent).toBe( + "An error occurred. Press Alt+ArrowDown for error details", + ); + expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); }, ); - it("localizes the generic error and accessible popup labels", async () => { - const { input, user } = renderEditor( + it("passes unexpected errors unchanged to custom renderers without leaking them into accessible feedback", async () => { + const failure = new Error("Internal server details"); + const renderError = vi.fn(() =>

    Custom feedback

    ); + const { input } = renderEditor({ + onValueCommitted: () => { + throw failure; + }, + error: { render: renderError }, + }); + change(input); + fireEvent.keyDown(input, { key: "Enter" }); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + expect(popup.textContent).toBe("Custom feedback"); + expect(renderError).toHaveBeenCalledWith({ row: ROWS[0], error: failure }); + expect(document.getElementById(input.getAttribute("aria-describedby")!)?.textContent).toBe( + "An error occurred. Press Alt+ArrowDown for error details", + ); + }); + + it.each([ + { failure: new Error("Internal server details"), message: "エラーが発生しました" }, + { failure: new CellEditError("数量を入力してください"), message: "数量を入力してください" }, + ])("localizes popup labels and displays $message", async ({ failure, message }) => { + const { input } = renderEditor( { onValueChange: () => { - throw undefined; + throw failure; }, onValueCommitted: noop, }, { locale: "ja" }, ); change(input); - await user.click(screen.getByRole("button", { name: "Valueのエラーを表示" })); - expect( - within(await screen.findByRole("dialog", { name: "Valueのエラー" })).getByText( - "エラーが発生しました", - ), - ).toBeDefined(); + const popup = await screen.findByRole("dialog", { name: "Valueのエラー" }); + expect(within(popup).getByText(message)).toBeDefined(); + expect(within(popup).getByRole("button", { name: "エラーの詳細を閉じる" })).toBeDefined(); + expect(document.getElementById(input.getAttribute("aria-describedby")!)?.textContent).toBe( + `${message}. Alt+↓キーでエラーの詳細を表示できます`, + ); }); it("clears an error on a newer successful input and ignores older rejection results", async () => { @@ -261,7 +403,7 @@ describe("DataTable callback error presentation", () => { }); change(input, "Older"); fireEvent.change(input, { target: { value: "Current" } }); - await rejectRequest(current, new Error("Current failure")); + await rejectRequest(current, new CellEditError("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( @@ -338,20 +480,29 @@ describe("DataTable callback error presentation", () => { expect(onValueCommitted).toHaveBeenCalledTimes(1); }); - it("retains a failed confirmation through blur and refocus, then permits an Enter retry", async () => { + it("retains an async blur failure through refocus and unchanged blur, then permits an Enter retry", async () => { + const save = deferred(); let rejected = true; - const onValueCommitted = vi.fn(() => { - if (rejected) throw new Error("Temporary save failure"); - }); + const onValueCommitted = vi.fn(() => (rejected ? save.promise : undefined)); const { input, user } = renderEditor({ onValueCommitted }); + const outside = screen.getByRole("button", { name: "Outside" }); change(input); - fireEvent.keyDown(input, { key: "Enter" }); - await user.click(screen.getByRole("button", { name: "Outside" })); + await user.click(outside); + expect(onValueCommitted).toHaveBeenCalledExactlyOnceWith(ROWS[0], "Changed"); + await rejectRequest(save, new CellEditError("Temporary save failure")); expect(input.value).toBe("Changed"); expect(input.getAttribute("aria-invalid")).toBe("true"); - expect(onValueCommitted).toHaveBeenCalledTimes(1); await user.click(input); + expect(input.getAttribute("aria-invalid")).toBe("true"); + await user.click(outside); expect(input.value).toBe("Changed"); + expect(input.getAttribute("aria-invalid")).toBe("true"); + expect(document.getElementById(input.getAttribute("aria-describedby")!)?.textContent).toContain( + "Temporary save failure", + ); + expect(queryErrorTrigger()).not.toBeNull(); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + await user.click(input); rejected = false; fireEvent.keyDown(input, { key: "Enter" }); expect(onValueCommitted).toHaveBeenCalledTimes(2); @@ -376,6 +527,8 @@ describe("DataTable callback error presentation", () => { expect(input.value).toBe("１３.５０"); expect(input.getAttribute("aria-invalid")).toBe("true"); fireEvent.keyDown(input, { key: "Escape" }); + expect(input.value).toBe("１３.５０"); + fireEvent.keyDown(input, { key: "Escape" }); expect(input.value).toBe("12"); expect(input.getAttribute("aria-invalid")).toBeNull(); expect(queryErrorTrigger()).toBeNull(); @@ -394,6 +547,8 @@ describe("DataTable callback error presentation", () => { expect(input.value).toBe("Changed"); expect(input.getAttribute("aria-invalid")).toBe("true"); fireEvent.keyDown(input, { key: "Escape" }); + expect(input.value).toBe("Changed"); + fireEvent.keyDown(input, { key: "Escape" }); expect(input.value).toBe("Updated externally"); expect(input.readOnly).toBe(true); expect(input.getAttribute("aria-invalid")).toBeNull(); @@ -403,24 +558,45 @@ describe("DataTable callback error presentation", () => { expect(onValueCommitted).toHaveBeenCalledTimes(1); }); - it("uses the first Escape to close error details and the next Escape to cancel the failed draft", async () => { + it("supports keyboard error inspection, focus return, Tab navigation, and Escape cancellation", async () => { const onValueCommitted = vi.fn(() => { - throw new Error("Not accepted"); + throw new CellEditError("Not accepted"); }); - const { input, user } = renderEditor({ onValueCommitted }); + const { input, inputs, user } = renderEditor( + { onValueCommitted }, + { rows: [...ROWS, { id: "2", value: "Second" }] }, + ); 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("{Enter}"); + expect(input.getAttribute("aria-keyshortcuts")).toBe("Alt+ArrowDown"); + expect(document.getElementById(input.getAttribute("aria-describedby")!)?.textContent).toContain( + "Press Alt+ArrowDown for error details", + ); + await user.keyboard("{Alt>}{ArrowDown}{/Alt}"); + const popup = await screen.findByRole("dialog", { name: "Error in Value" }); + expect(within(popup).getByText("Not accepted")).toBeDefined(); + expect(document.activeElement).toBe(popup); await user.keyboard("{Escape}"); await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + await vi.waitFor(() => + expect(input.closest("td")?.contains(document.activeElement)).toBe(true), + ); expect(input.value).toBe("Changed"); expect(input.getAttribute("aria-invalid")).toBe("true"); + expect(onValueCommitted).toHaveBeenCalledTimes(1); + await user.tab(); + expect(document.activeElement).toBe(inputs[1]); + expect(onValueCommitted).toHaveBeenCalledTimes(2); + expect(input.getAttribute("aria-invalid")).toBe("true"); + await user.tab({ shift: true }); + expect(document.activeElement).toBe(input); + await user.keyboard("{Escape}"); + expect(input.value).toBe("Changed"); await user.keyboard("{Escape}"); expect(input.value).toBe("Original"); expect(input.getAttribute("aria-invalid")).toBeNull(); expect(queryErrorTrigger()).toBeNull(); - expect(onValueCommitted).toHaveBeenCalledTimes(1); + expect(onValueCommitted).toHaveBeenCalledTimes(2); }); it("clears cancellation errors and ignores pending results after Escape", async () => { @@ -481,7 +657,7 @@ describe("DataTable callback error presentation", () => { "retains a rejected %s choice through error inspection until cancellation", async (type) => { const onValueCommitted = vi.fn(() => { - throw new Error("Choice rejected"); + throw new CellEditError("Choice rejected"); }); const user = renderColumn({ id: "value", @@ -498,7 +674,6 @@ describe("DataTable callback error presentation", () => { 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()); @@ -650,7 +825,7 @@ describe("DataTable callback error presentation", () => { typeOptions: { dateFormat: "datetime" }, edit: { onValueChange: () => { - throw new Error("Time rejected"); + throw new CellEditError("Time rejected"); }, onValueCommitted, }, @@ -658,7 +833,6 @@ describe("DataTable callback error presentation", () => { 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); diff --git a/packages/core/src/components/data-table/editable-cell.tsx b/packages/core/src/components/data-table/editable-cell.tsx index c4f447359..28e22ba85 100644 --- a/packages/core/src/components/data-table/editable-cell.tsx +++ b/packages/core/src/components/data-table/editable-cell.tsx @@ -16,7 +16,7 @@ import { 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, CircleAlert } from "lucide-react"; +import { CalendarDays, ChevronDown, CircleAlert, X } from "lucide-react"; import { cn } from "@/lib/utils"; import { isComposing } from "@/lib/is-composing"; import { useTimeZone } from "@/contexts/appshell-context"; @@ -29,6 +29,7 @@ import { Table } from "@/components/table"; import { Tooltip } from "@/components/tooltip"; import type { Column, DateTimeDraft } from "./types"; import { getCellValue, renderTypedValue } from "./cell-renderers"; +import { CellEditError } from "./cell-edit-error"; import { normalizeNumberText, parseNumberText, @@ -128,7 +129,7 @@ const CELL_FOCUS_CLASS_NAME = cn( // Parsing and callback errors share the cell's error outline. const CELL_ERROR_CLASS_NAME = - "astw:ring-[1.5px] astw:ring-inset astw:ring-destructive astw:focus:ring-[1.5px] astw:focus:ring-destructive astw:focus-visible:ring-[1.5px] astw:focus-visible:ring-destructive"; + "astw:ring-[1.5px] astw:ring-inset astw:ring-danger-solid astw:focus:ring-[1.5px] astw:focus:ring-danger-solid astw:focus-visible:ring-[1.5px] astw:focus-visible:ring-danger-solid"; // An editable cell says so at rest with a subtle tint instead of an icon. It's // a background *image* layered over whatever colour the cell already has (the @@ -228,7 +229,7 @@ function useCellValue>( const errorConfig = col.edit?.error; const renderError = errorConfig && errorConfig.render; const message = - failure?.error instanceof Error && failure.error.message + failure?.error instanceof CellEditError && failure.error.message ? failure.error.message : t("editCallbackError"); const callbackError: CellErrorUI | undefined = @@ -304,7 +305,9 @@ function EditableCellFrame({ }) { const t = useDataTableT(); const [tooltipOpen, setTooltipOpen] = useState(false); - const [openFailure, setOpenFailure] = useState(null); + const [dismissedFailure, setDismissedFailure] = useState(null); + const [manuallyOpenedFailure, setManuallyOpenedFailure] = useState(null); + const errorOpen = !!callbackError && dismissedFailure !== callbackError.failure; const showTooltip = tooltipLabel !== undefined && !editing && !callbackError; const cell = ( { - if (callbackError && event.key === "Escape" && openFailure !== callbackError.failure) { + if ( + callbackError && + event.key === "Escape" && + !callbackError.popupRef.current?.contains(event.target as Node) + ) { event.preventDefault(); event.stopPropagation(); - onEditorCancel(); + if (errorOpen) setDismissedFailure(callbackError.failure); + else onEditorCancel(); return; } if (callbackError && event.altKey && event.key === "ArrowDown") { event.preventDefault(); event.stopPropagation(); - setOpenFailure(callbackError.failure); + setManuallyOpenedFailure(callbackError.failure); + setDismissedFailure(null); + if (errorOpen) callbackError.popupRef.current?.focus({ preventScroll: true }); } }} // Editing a cell must never fire `onClickRow`. @@ -365,15 +375,18 @@ function EditableCellFrame({ {callbackError.message}. {t("editErrorHint")} setOpenFailure(open ? callbackError.failure : null)} + open={errorOpen} + onOpenChange={(open) => { + setDismissedFailure(open ? null : callbackError.failure); + if (open) setManuallyOpenedFailure(callbackError.failure); + }} > event.stopPropagation()} >