diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index c534fb29..a381add1 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -1,3 +1,4 @@ +import { useState } from "react"; import { describe, it, expect, vi } from "vitest"; import userEvent from "@testing-library/user-event"; import type { InspectorFormSchema } from "../../../utils/jsonUtils"; @@ -228,6 +229,140 @@ describe("SchemaForm", () => { expect(lastCall.count).toBeUndefined(); }); + describe("number fields (#1888)", () => { + // The real callers keep the form values in state and feed them back in, so + // the bug only reproduces against a genuinely controlled SchemaForm: an + // uncontrolled render never rewrites the box and would pass either way. + function ControlledSchemaForm({ + schema, + initialValues = {}, + onChange, + }: { + schema: InspectorFormSchema; + initialValues?: Record; + onChange: (values: Record) => void; + }) { + const [values, setValues] = + useState>(initialValues); + return ( + { + setValues(next); + onChange(next); + }} + /> + ); + } + + const numberSchema: InspectorFormSchema = { + type: "object", + properties: { + divisor: { type: "number", title: "Divisor" }, + }, + }; + + it("lets a decimal be typed all the way through", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Divisor/) as HTMLInputElement; + await user.type(input, "1.5"); + expect(input.value).toBe("1.5"); + expect(onChange).toHaveBeenLastCalledWith({ divisor: 1.5 }); + }); + + it("keeps the trailing decimal point visible mid-entry", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Divisor/) as HTMLInputElement; + await user.type(input, "1."); + // The point survives on screen even though "1." parses to plain 1. + expect(input.value).toBe("1."); + expect(onChange).toHaveBeenLastCalledWith({ divisor: 1 }); + }); + + it("keeps a trailing zero after the decimal point", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Divisor/) as HTMLInputElement; + await user.type(input, "1.50"); + expect(input.value).toBe("1.50"); + expect(onChange).toHaveBeenLastCalledWith({ divisor: 1.5 }); + }); + + it("reports a lone minus sign as no value while leaving it typed", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Divisor/) as HTMLInputElement; + await user.type(input, "-"); + expect(input.value).toBe("-"); + expect(onChange).toHaveBeenLastCalledWith({ divisor: undefined }); + await user.type(input, "2.5"); + expect(onChange).toHaveBeenLastCalledWith({ divisor: -2.5 }); + }); + + it("rejects a decimal point in an integer field", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + const schema: InspectorFormSchema = { + type: "object", + properties: { + count: { type: "integer", title: "Count" }, + }, + }; + renderWithMantine( + , + ); + const input = screen.getByLabelText(/Count/) as HTMLInputElement; + await user.type(input, "1.5"); + expect(input.value).toBe("15"); + expect(onChange).toHaveBeenLastCalledWith({ count: 15 }); + }); + + it("re-syncs the displayed text when the value changes externally", async () => { + const user = userEvent.setup(); + const onChange = vi.fn(); + function Harness() { + const [values, setValues] = useState>({ + divisor: 1.5, + }); + return ( + <> + + { + setValues(next); + onChange(next); + }} + /> + + ); + } + renderWithMantine(); + const input = screen.getByLabelText(/Divisor/) as HTMLInputElement; + expect(input.value).toBe("1.5"); + await user.click(screen.getByRole("button", { name: "Load example" })); + expect(input.value).toBe("42"); + }); + }); + it("falls back to empty/const labels for oneOf items missing const and title", () => { const onChange = vi.fn(); const schema: InspectorFormSchema = { diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 30e39795..0e67d854 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -8,7 +8,9 @@ import { Text, TextInput, } from "@mantine/core"; +import { useState } from "react"; import { ClearButton } from "../../elements/ClearButton/ClearButton"; +import { useValueChange } from "../../../hooks/useValueChange"; import type { InspectorFormSchema } from "../../../utils/jsonUtils"; const FieldLabel = Text.withProps({ @@ -49,6 +51,79 @@ function toEnumData( return values; } +/** + * Interpret whatever Mantine's `NumberInput` reported as the JSON value for the + * field. It emits a `number` once the text parses cleanly, and the **raw string** + * while it does not — `""` when cleared, but also the in-progress `"1."`, `"-"`, + * and `"1e"`. Anything that is not a finite number becomes `undefined`, which is + * how an absent optional argument is represented everywhere else in this form. + */ +function toNumericValue(raw: string | number): number | undefined { + if (typeof raw === "number") { + return Number.isFinite(raw) ? raw : undefined; + } + if (raw.trim() === "") { + return undefined; + } + const parsed = Number(raw); + return Number.isFinite(parsed) ? parsed : undefined; +} + +interface SchemaNumberInputProps { + label: string; + description?: string; + withAsterisk: boolean; + disabled: boolean; + value: number | undefined; + min?: number; + max?: number; + allowDecimal: boolean; + onChange: (value: number | undefined) => void; +} + +/** + * A `NumberInput` that keeps the text the user is typing, not just the number it + * currently parses to. + * + * Driving `NumberInput` directly off the parent's numeric value makes a decimal + * impossible to enter (#1888): typing `.` after `1` produces the unparseable + * string `"1."`, the numeric value stays `1`, and the controlled `value` prop + * immediately rewrites the box back to `"1"` — so the `.` vanishes and `1.5` can + * never be reached. Trailing zeros (`"1.50"`) and a lone leading `"-"` fail the + * same way. + * + * So the raw text is held here as the source of truth for what is *displayed*, + * while the parent still only ever sees a `number | undefined`. The two are + * re-synced only when the parent's value genuinely diverges from what the draft + * parses to, which leaves an external reset (a cleared form, a loaded example) + * working while an in-progress `"1."` — whose parse is `1`, matching the value we + * just emitted — is left alone. + */ +function SchemaNumberInput({ + value, + onChange, + ...inputProps +}: SchemaNumberInputProps) { + const [draft, setDraft] = useState(value ?? ""); + + useValueChange(value, (next) => { + if (!Object.is(toNumericValue(draft), next)) { + setDraft(next ?? ""); + } + }); + + return ( + { + setDraft(next); + onChange(toNumericValue(next)); + }} + /> + ); +} + export interface SchemaFormProps { schema: InspectorFormSchema; values: Record; @@ -156,19 +231,19 @@ export function SchemaForm({ // number or integer if (fieldSchema.type === "number" || fieldSchema.type === "integer") { return ( - { - const numericValue = typeof val === "string" ? undefined : val; - handleFieldChange(fieldName, numericValue); - }} + // An `integer` field rejects the decimal point outright rather than + // accepting a value the schema forbids. + allowDecimal={fieldSchema.type === "number"} + onChange={(val) => handleFieldChange(fieldName, val)} /> ); }