From e0ecf58df3dba2be56ba6a619fa4a6f4cff09914 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 26 Aug 2026 16:25:57 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20address=20Copilot=20review=20round=2042?= =?UTF-8?q?=20=E2=80=94=20let=20an=20optional=20const=20be=20opted=20into?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Making every pinned field read-only left an optional one unreachable: the schema is equally happy with or without it, but the form displayed a value the user could neither send nor edit. An optional `const` is a yes/no rather than a fixed answer, so it renders as its single choice, clearable — opting in sends the schema's own typed value, opting out leaves the property absent. A required one is still displayed read-only, since there is nothing to decide. Signed-off-by: cliffhall --- .../groups/SchemaForm/SchemaForm.test.tsx | 42 +++++++++++++++++++ .../groups/SchemaForm/SchemaForm.tsx | 42 ++++++++++++++++--- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index b0c24438..826f759d 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -2054,6 +2054,46 @@ describe("SchemaForm multiline strings (#2042)", () => { expect(kind.value).toBe("email"); }); + it("lets an optional const be opted into and left out", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + const optional: InspectorFormSchema = { + type: "object", + properties: { dryRun: { type: "boolean", const: true, title: "Dry" } }, + }; + function Host() { + const [values, setValues] = useState>({}); + return ( + { + setValues(next); + onChange(next); + }} + /> + ); + } + renderWithMantine(); + + const field = screen.getByRole("textbox", { + name: /Dry/, + }) as HTMLInputElement; + // Not supplied to begin with — `const` does not demand the property. + expect(field.value).toBe(""); + + await user.click(field); + await user.click(screen.getByRole("option", { name: "true" })); + // Opting in sends the schema's own typed value, not the label. + expect(onChange).toHaveBeenCalledWith({ dryRun: true }); + + // Mantine marks its combobox clear button `aria-hidden` (mouse-only, + // `tabIndex={-1}`), so it is only reachable with `hidden: true`. + await user.click(screen.getByRole("button", { hidden: true })); + // …and opting back out leaves the property absent from the call. + expect(onChange).toHaveBeenLastCalledWith({ dryRun: undefined }); + }); + it("renders a non-string constant read-only too", () => { // Reached before the number/boolean dispatch, so neither offers a value // the `const` forbids. @@ -2065,6 +2105,7 @@ describe("SchemaForm multiline strings (#2042)", () => { n: { type: "number", const: 7, title: "N" }, b: { type: "boolean", const: true, title: "B" }, }, + required: ["n", "b"], }} values={{}} onChange={vi.fn()} @@ -2093,6 +2134,7 @@ describe("SchemaForm multiline strings (#2042)", () => { title: "Mode", }, }, + required: ["mode"], }} values={{}} onChange={vi.fn()} diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 9fbb3d69..0f1468a3 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -877,19 +877,49 @@ export function SchemaForm({ // constant keeps its type on the wire however it is shown here. if (fieldSchema.const !== undefined) { const constValue = fieldSchema.const; + const constText = + typeof constValue === "string" ? constValue : serializeJson(constValue); + + // An OPTIONAL pinned field is a yes/no, not a fixed answer: `const` + // constrains the value if the property is there, and the schema is + // equally happy without it. So it gets the one choice it has, clearable — + // the user can opt in or leave it out, and cannot type anything else. + if (!isRequired) { + return ( +