From d3b70a6329bee2c65cd9c25d01fa7237537c3c9e Mon Sep 17 00:00:00 2001 From: cliffhall Date: Sun, 16 Aug 2026 00:53:16 -0400 Subject: [PATCH] fix(web): keep an explicit null out of the number field's default fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review: seeding with `value ?? defaultValue` also substitutes the default when the raw value is an explicit `null` — a nullable number field the user cleared, or one the server sent — where `resolveValue` previously kept the null and the box rendered empty. A nullable schema with a non-null default would have opened showing that default over a real value. Test the `undefined` case explicitly, as `SchemaJsonField` already does, and pin it: the new test fails with `'30'` where `''` belongs. Signed-off-by: cliffhall --- .../groups/SchemaForm/SchemaForm.test.tsx | 19 +++++++++++++++++++ .../groups/SchemaForm/SchemaForm.tsx | 7 ++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index e963ff8c..3c7193df 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -1742,6 +1742,25 @@ describe("SchemaForm defaulted fields (#2026)", () => { expect(n.value).toBe(""); }); + // An explicit `null` is a value, not an absent one, so a non-null default + // must not be substituted for it. Reachable for a nullable number field the + // user cleared, or one the server sent as null. + it("shows an explicit null as empty, not as the default", () => { + const schema: InspectorFormSchema = { + type: "object", + properties: { + n: { + title: "N", + anyOf: [{ type: "integer" }, { type: "null" }], + default: 30, + }, + }, + }; + renderWithMantine(); + + expect((screen.getByLabelText(/^N$/) as HTMLInputElement).value).toBe(""); + }); + it("still re-syncs a defaulted field when the value changes externally", async () => { function ExternalHarness() { const [values, setValues] = useState>({ diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 53edeeca..acd59ca7 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -379,8 +379,13 @@ function SchemaNumberInput({ onValidityChange, ...inputProps }: SchemaNumberInputProps) { + // `undefined` means "no value supplied", which is what the default is for. + // An explicit `null` is a value — a nullable number the user cleared, or one + // the server sent — so it must not be overwritten by a non-null default; it + // displays as the empty box `resolveValue` produced before. Hence the + // `undefined` test rather than `??`, matching `SchemaJsonField`. const [draft, setDraft] = useState( - value ?? defaultValue ?? "", + (value === undefined ? defaultValue : value) ?? "", ); // Compared against the raw value, never the default-substituted one, for the