diff --git a/frontend/src/components/InlineEditCell.test.tsx b/frontend/src/components/InlineEditCell.test.tsx new file mode 100644 index 0000000..90e328f --- /dev/null +++ b/frontend/src/components/InlineEditCell.test.tsx @@ -0,0 +1,57 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, fireEvent } from "@solidjs/testing-library"; +import { createSignal } from "solid-js"; + +vi.mock("./Toast", () => ({ showToast: vi.fn() })); + +import InlineEditCell from "./InlineEditCell"; + +function flush() { + return new Promise((r) => setTimeout(r, 0)); +} + +describe("InlineEditCell boolean", () => { + it("keeps the new checked state after save resolves even if props.value is stale", async () => { + // Parent does not refetch after save, so props.value stays at the old value. + const onSave = vi.fn().mockResolvedValue(undefined); + const { container } = render(() => ( + + )); + const box = container.querySelector("input[type=checkbox]") as HTMLInputElement; + expect(box.checked).toBe(false); + + fireEvent.click(box); + await flush(); + + expect(onSave).toHaveBeenCalledWith("true"); + // Bug: setSaving(false) re-ran the sync effect with the stale prop and + // reverted the checkbox to unchecked. + expect(box.checked).toBe(true); + }); + + it("still syncs when props.value actually changes from outside", async () => { + const [value, setValue] = createSignal("false"); + const { container } = render(() => ( + + )); + const box = container.querySelector("input[type=checkbox]") as HTMLInputElement; + expect(box.checked).toBe(false); + + setValue("true"); + await flush(); + expect(box.checked).toBe(true); + }); + + it("reverts to the previous value when save fails", async () => { + const onSave = vi.fn().mockRejectedValue(new Error("boom")); + const { container } = render(() => ( + + )); + const box = container.querySelector("input[type=checkbox]") as HTMLInputElement; + + fireEvent.click(box); + await flush(); + + expect(box.checked).toBe(false); + }); +}); diff --git a/frontend/src/components/InlineEditCell.tsx b/frontend/src/components/InlineEditCell.tsx index cb66308..cbe96fb 100644 --- a/frontend/src/components/InlineEditCell.tsx +++ b/frontend/src/components/InlineEditCell.tsx @@ -1,4 +1,4 @@ -import { createSignal, Show, For, createEffect } from "solid-js"; +import { createSignal, Show, For, createEffect, on, untrack } from "solid-js"; import { showToast } from "./Toast"; import { getErrorMessage } from "../utils/errors"; @@ -16,11 +16,15 @@ export default function InlineEditCell(props: Props) { const [saving, setSaving] = createSignal(false); // Sync from props when external data changes (e.g. refetch), but never - // stomp on an in-flight save that has not yet resolved. - createEffect(() => { - const incoming = props.value; - if (!saving()) setLocalValue(incoming); - }); + // stomp on an in-flight save that has not yet resolved. Track only + // props.value: tracking saving() too would re-run this when a save + // finishes and clobber the just-committed value with a stale prop. + createEffect(on( + () => props.value, + (incoming) => { + if (!untrack(saving)) setLocalValue(incoming); + }, + )); // Confirm-then-commit: keep the old value visible (with a spinner) until the // save resolves. On success show the new value; on failure keep the old value @@ -56,12 +60,9 @@ export default function InlineEditCell(props: Props) { if (e.key === "Escape") setEditing(false); } - const Spinner = () => ( - - ); + // Saving feedback is the greyed-out disabled control itself; anything that + // mounts next to it (spinner) flashes on fast LAN saves and reads as a + // rendering artifact. // Boolean: simple checkbox if (props.columnType === "boolean") { @@ -71,10 +72,19 @@ export default function InlineEditCell(props: Props) { type="checkbox" checked={localValue() === "true"} disabled={saving()} - onChange={(e) => void save(e.currentTarget.checked ? "true" : "false")} - class="cursor-pointer disabled:opacity-50" + onChange={(e) => { + const el = e.currentTarget; + const want = el.checked ? "true" : "false"; + // Confirm-then-commit: snap the DOM back to the committed value; + // a successful save flips localValue and re-checks it. Without + // this a failed save leaves the DOM out of sync, since the + // unchanged signal never rewrites the checked attribute. + el.checked = localValue() === "true"; + void save(want); + }} + class="cursor-pointer disabled:opacity-40" + title={saving() ? "Saving..." : undefined} /> - ); } @@ -90,14 +100,14 @@ export default function InlineEditCell(props: Props) { const val = e.currentTarget.value; if (val) void save(val); }} - class="px-1 py-0.5 rounded border border-theme-border bg-theme-input text-theme-text-primary text-sm disabled:opacity-50" + class="px-1 py-0.5 rounded border border-theme-border bg-theme-input text-theme-text-primary text-sm disabled:opacity-40" + title={saving() ? "Saving..." : undefined} > {(opt) => } - ); } @@ -110,11 +120,10 @@ export default function InlineEditCell(props: Props) { {localValue() || "-"} - } >