fix(web): keep the text being typed in a schema number field (#1888)
`SchemaForm` drove Mantine's `NumberInput` directly off the parent's numeric value. `NumberInput` reports the raw string while the text does not parse, so pressing `.` after `1` produced `"1."`, which the handler collapsed to `undefined` — and because the control is fully controlled, the `value` prop immediately rewrote the box back to `1`. The decimal point was discarded on every keystroke, making `1.5` unreachable. A trailing zero, a lone leading `-`, and an exponent failed the same way. Introduce `SchemaNumberInput`, which holds the in-progress text as the source of truth for what is displayed while the parent still only ever sees a `number | undefined`. Draft and value re-sync only when they genuinely diverge, via `useValueChange` (adjust-state-during-render) rather than an effect, so an external reset still rewrites the box but an in-progress `"1."` is left alone. Also pass `allowDecimal` from the schema, so an `integer` field rejects the decimal point instead of accepting a value its schema forbids — matching what the TUI already does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0119LaPQ3v4NyBK3ZTdM9j84
This commit is contained in:
parent
a09f2dcf1d
commit
51bbe9996b
2 changed files with 216 additions and 6 deletions
|
|
@ -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<string, unknown>;
|
||||
onChange: (values: Record<string, unknown>) => void;
|
||||
}) {
|
||||
const [values, setValues] =
|
||||
useState<Record<string, unknown>>(initialValues);
|
||||
return (
|
||||
<SchemaForm
|
||||
schema={schema}
|
||||
values={values}
|
||||
onChange={(next) => {
|
||||
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(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
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(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
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(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
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(
|
||||
<ControlledSchemaForm schema={numberSchema} onChange={onChange} />,
|
||||
);
|
||||
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(
|
||||
<ControlledSchemaForm schema={schema} onChange={onChange} />,
|
||||
);
|
||||
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<Record<string, unknown>>({
|
||||
divisor: 1.5,
|
||||
});
|
||||
return (
|
||||
<>
|
||||
<button type="button" onClick={() => setValues({ divisor: 42 })}>
|
||||
Load example
|
||||
</button>
|
||||
<SchemaForm
|
||||
schema={numberSchema}
|
||||
values={values}
|
||||
onChange={(next) => {
|
||||
setValues(next);
|
||||
onChange(next);
|
||||
}}
|
||||
/>
|
||||
</>
|
||||
);
|
||||
}
|
||||
renderWithMantine(<Harness />);
|
||||
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 = {
|
||||
|
|
|
|||
|
|
@ -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<string | number>(value ?? "");
|
||||
|
||||
useValueChange(value, (next) => {
|
||||
if (!Object.is(toNumericValue(draft), next)) {
|
||||
setDraft(next ?? "");
|
||||
}
|
||||
});
|
||||
|
||||
return (
|
||||
<NumberInput
|
||||
{...inputProps}
|
||||
value={draft}
|
||||
onChange={(next) => {
|
||||
setDraft(next);
|
||||
onChange(toNumericValue(next));
|
||||
}}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
export interface SchemaFormProps {
|
||||
schema: InspectorFormSchema;
|
||||
values: Record<string, unknown>;
|
||||
|
|
@ -156,19 +231,19 @@ export function SchemaForm({
|
|||
// number or integer
|
||||
if (fieldSchema.type === "number" || fieldSchema.type === "integer") {
|
||||
return (
|
||||
<NumberInput
|
||||
<SchemaNumberInput
|
||||
key={fieldName}
|
||||
label={label}
|
||||
description={description}
|
||||
withAsterisk={isRequired}
|
||||
disabled={disabled}
|
||||
value={(rawValue as number) ?? ""}
|
||||
value={rawValue as number | undefined}
|
||||
min={fieldSchema.minimum}
|
||||
max={fieldSchema.maximum}
|
||||
onChange={(val) => {
|
||||
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)}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue