fix: address Copilot review round 42 — let an optional const be opted into
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 <cliff@futurescale.com>
This commit is contained in:
parent
b0b252a5c0
commit
e0ecf58df3
2 changed files with 78 additions and 6 deletions
|
|
@ -2054,6 +2054,46 @@ describe("SchemaForm multiline strings (#2042)", () => {
|
||||||
expect(kind.value).toBe("email");
|
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<Record<string, unknown>>({});
|
||||||
|
return (
|
||||||
|
<SchemaForm
|
||||||
|
schema={optional}
|
||||||
|
values={values}
|
||||||
|
onChange={(next) => {
|
||||||
|
setValues(next);
|
||||||
|
onChange(next);
|
||||||
|
}}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
renderWithMantine(<Host />);
|
||||||
|
|
||||||
|
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", () => {
|
it("renders a non-string constant read-only too", () => {
|
||||||
// Reached before the number/boolean dispatch, so neither offers a value
|
// Reached before the number/boolean dispatch, so neither offers a value
|
||||||
// the `const` forbids.
|
// the `const` forbids.
|
||||||
|
|
@ -2065,6 +2105,7 @@ describe("SchemaForm multiline strings (#2042)", () => {
|
||||||
n: { type: "number", const: 7, title: "N" },
|
n: { type: "number", const: 7, title: "N" },
|
||||||
b: { type: "boolean", const: true, title: "B" },
|
b: { type: "boolean", const: true, title: "B" },
|
||||||
},
|
},
|
||||||
|
required: ["n", "b"],
|
||||||
}}
|
}}
|
||||||
values={{}}
|
values={{}}
|
||||||
onChange={vi.fn()}
|
onChange={vi.fn()}
|
||||||
|
|
@ -2093,6 +2134,7 @@ describe("SchemaForm multiline strings (#2042)", () => {
|
||||||
title: "Mode",
|
title: "Mode",
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
required: ["mode"],
|
||||||
}}
|
}}
|
||||||
values={{}}
|
values={{}}
|
||||||
onChange={vi.fn()}
|
onChange={vi.fn()}
|
||||||
|
|
|
||||||
|
|
@ -877,19 +877,49 @@ export function SchemaForm({
|
||||||
// constant keeps its type on the wire however it is shown here.
|
// constant keeps its type on the wire however it is shown here.
|
||||||
if (fieldSchema.const !== undefined) {
|
if (fieldSchema.const !== undefined) {
|
||||||
const constValue = fieldSchema.const;
|
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 (
|
||||||
|
<Select
|
||||||
|
key={fieldName}
|
||||||
|
label={label}
|
||||||
|
description={description}
|
||||||
|
disabled={disabled}
|
||||||
|
data={[{ value: constText, label: constText }]}
|
||||||
|
clearable
|
||||||
|
value={
|
||||||
|
Object.hasOwn(values, fieldName) &&
|
||||||
|
values[fieldName] !== undefined
|
||||||
|
? constText
|
||||||
|
: null
|
||||||
|
}
|
||||||
|
onChange={(picked) =>
|
||||||
|
handleFieldChange(
|
||||||
|
fieldName,
|
||||||
|
picked === null ? undefined : constValue,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Required: there is nothing to decide, so it is displayed and not
|
||||||
|
// editable. The value itself is seeded by `collectSchemaDefaults`.
|
||||||
return (
|
return (
|
||||||
<TextInput
|
<TextInput
|
||||||
key={fieldName}
|
key={fieldName}
|
||||||
label={label}
|
label={label}
|
||||||
description={description}
|
description={description}
|
||||||
withAsterisk={isRequired}
|
withAsterisk
|
||||||
readOnly
|
readOnly
|
||||||
disabled={disabled}
|
disabled={disabled}
|
||||||
value={
|
value={constText}
|
||||||
typeof constValue === "string"
|
|
||||||
? constValue
|
|
||||||
: serializeJson(constValue)
|
|
||||||
}
|
|
||||||
/>
|
/>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue