From 4513d183b120998af2efa96bc8c88fb608de578c Mon Sep 17 00:00:00 2001 From: alfredorubin96 Date: Tue, 19 May 2026 02:29:52 +0200 Subject: [PATCH 1/3] fix(number-range): draft inputs, NaN guards, validation (#857) The number-range slider added in #853 had several rough edges in the input + value pipeline that surfaced once the dual-thumb primitive landed. Fixes: - NumberRangeSlider min/max inputs now track a draft string and commit on blur/Enter. Previously `Number("-")` was NaN and silently no-op'd, leaving the field dead while typing a negative. - handleSliderChange falls back to a sane second value when Radix emits a single-element array (min===max edge case), so the tuple is always [number, number]. - param-number-range.tsx now drops non-finite tuples from the restore path instead of producing [NaN, NaN] slider state. - form-fields-editor number-range section refactored into its own component with draft state. Empty clear no longer silently zeros rangeMin via `Number("") === 0`. step <= 0 keeps the prior value. - Inline validation error when min >= max or step <= 0. Tests: - New draft-and-blur behavior coverage on the slider + editor. - NaN-guard coverage on the restore path. - Min/max validation message coverage. --- .../__tests__/param-number-range.test.ts | 55 ++- .../parameters/param-number-range.tsx | 13 +- .../__tests__/form-fields-editor.test.tsx | 142 +++++++ .../widget-editor/form-fields-editor.tsx | 203 +++++++--- .../__tests__/parameter-widgets.test.tsx | 382 ++++++++++++------ .../parameter-widgets/number-range-slider.tsx | 74 +++- 6 files changed, 644 insertions(+), 225 deletions(-) diff --git a/app/src/components/parameters/__tests__/param-number-range.test.ts b/app/src/components/parameters/__tests__/param-number-range.test.ts index 8e2c7198..35f505e7 100644 --- a/app/src/components/parameters/__tests__/param-number-range.test.ts +++ b/app/src/components/parameters/__tests__/param-number-range.test.ts @@ -82,30 +82,43 @@ describe("ParamNumberRange — store interactions", () => { describe("ParamNumberRange — value coercion", () => { beforeEach(resetStore); + // Mirror the NaN-guarded parsing in param-number-range.tsx — kept as a tiny + // inline helper so the table-driven cases below read top-to-bottom. + const parseRangeValue = (raw: unknown): [number, number] | null => { + if (Array.isArray(raw) && raw.length >= 2) { + const lo = Number(raw[0]); + const hi = Number(raw[1]); + if (Number.isFinite(lo) && Number.isFinite(hi)) return [lo, hi]; + } + return null; + }; + it("converts stored tuple to [number, number]", () => { - const { setParameter } = useParameterStore.getState(); - setParameter( - "price", - [100, 500], - "Parameter Selector", - "price", - "number-range", - "selector-widget", - ); - const currentEntry = useParameterStore.getState().parameters["price"]; - const rawRange = currentEntry?.value; - const rangeValue: [number, number] | null = Array.isArray(rawRange) - ? [Number(rawRange[0]), Number(rawRange[1])] - : null; - expect(rangeValue).toEqual([100, 500]); + expect(parseRangeValue([100, 500])).toEqual([100, 500]); }); it("returns null when no entry", () => { - const currentEntry = useParameterStore.getState().parameters["price"]; - const rawRange = currentEntry?.value; - const rangeValue: [number, number] | null = Array.isArray(rawRange) - ? [Number(rawRange[0]), Number(rawRange[1])] - : null; - expect(rangeValue).toBeNull(); + expect(parseRangeValue(undefined)).toBeNull(); + }); + + it("returns null for non-array values (corrupt restore)", () => { + expect(parseRangeValue("not-an-array")).toBeNull(); + expect(parseRangeValue(42)).toBeNull(); + expect(parseRangeValue({ from: 1, to: 2 })).toBeNull(); + }); + + it("returns null when tuple contains non-numeric entries", () => { + // Previous code returned [NaN, NaN]; guard now drops the value entirely. + expect(parseRangeValue(["foo", "bar"])).toBeNull(); + expect(parseRangeValue([undefined, 5])).toBeNull(); + }); + + it("returns null when array is too short", () => { + expect(parseRangeValue([5])).toBeNull(); + expect(parseRangeValue([])).toBeNull(); + }); + + it("accepts numeric strings (restored from JSON)", () => { + expect(parseRangeValue(["10", "20"])).toEqual([10, 20]); }); }); diff --git a/app/src/components/parameters/param-number-range.tsx b/app/src/components/parameters/param-number-range.tsx index 229b4065..f5227704 100644 --- a/app/src/components/parameters/param-number-range.tsx +++ b/app/src/components/parameters/param-number-range.tsx @@ -24,9 +24,16 @@ export function ParamNumberRange({ className, }: ParamNumberRangeProps) { const rawRange = actions.currentEntry?.value; - const rangeValue: [number, number] | null = Array.isArray(rawRange) - ? [Number(rawRange[0]), Number(rawRange[1])] - : null; + let rangeValue: [number, number] | null = null; + if (Array.isArray(rawRange) && rawRange.length >= 2) { + const lo = Number(rawRange[0]); + const hi = Number(rawRange[1]); + // Drop tuples that don't parse to finite numbers — a corrupt restore would + // otherwise leave the slider stuck at [NaN, NaN]. + if (Number.isFinite(lo) && Number.isFinite(hi)) { + rangeValue = [lo, hi]; + } + } const handleChange = (vals: [number, number]) => { const coerced: [number, number] = diff --git a/app/src/components/widget-editor/__tests__/form-fields-editor.test.tsx b/app/src/components/widget-editor/__tests__/form-fields-editor.test.tsx index a40877aa..d4f3c298 100644 --- a/app/src/components/widget-editor/__tests__/form-fields-editor.test.tsx +++ b/app/src/components/widget-editor/__tests__/form-fields-editor.test.tsx @@ -357,6 +357,7 @@ describe("FormFieldsEditor", () => { render(); const minInput = screen.getByDisplayValue("0"); fireEvent.change(minInput, { target: { value: "3.7" } }); + fireEvent.blur(minInput); const setCall = mockSetFormFields.mock.calls.at(-1)!; const next = setCall[0] as FormFieldDef[]; const updated = next.find((f) => f.id === "f1")!; @@ -380,6 +381,7 @@ describe("FormFieldsEditor", () => { render(); const minInput = screen.getByDisplayValue("0"); fireEvent.change(minInput, { target: { value: "2.5" } }); + fireEvent.blur(minInput); const setCall = mockSetFormFields.mock.calls.at(-1)!; const next = setCall[0] as FormFieldDef[]; const updated = next.find((f) => f.id === "f1")!; @@ -426,6 +428,7 @@ describe("FormFieldsEditor", () => { render(); const maxInput = screen.getByDisplayValue("10"); fireEvent.change(maxInput, { target: { value: "12.6" } }); + fireEvent.blur(maxInput); const next = mockSetFormFields.mock.calls.at(-1)![0] as FormFieldDef[]; expect(next.find((f) => f.id === "f1")!.rangeMax).toBe(13); }); @@ -447,6 +450,7 @@ describe("FormFieldsEditor", () => { render(); const maxInput = screen.getByDisplayValue("10"); fireEvent.change(maxInput, { target: { value: "7.25" } }); + fireEvent.blur(maxInput); const next = mockSetFormFields.mock.calls.at(-1)![0] as FormFieldDef[]; expect(next.find((f) => f.id === "f1")!.rangeMax).toBe(7.25); }); @@ -468,6 +472,7 @@ describe("FormFieldsEditor", () => { render(); const stepInput = screen.getByDisplayValue("5"); fireEvent.change(stepInput, { target: { value: "0.3" } }); + fireEvent.blur(stepInput); const next = mockSetFormFields.mock.calls.at(-1)![0] as FormFieldDef[]; // Math.max(1, Math.round(0.3)) === 1 expect(next.find((f) => f.id === "f1")!.rangeStep).toBe(1); @@ -490,9 +495,146 @@ describe("FormFieldsEditor", () => { render(); const stepInput = screen.getByDisplayValue("0.5"); fireEvent.change(stepInput, { target: { value: "0.25" } }); + fireEvent.blur(stepInput); const next = mockSetFormFields.mock.calls.at(-1)![0] as FormFieldDef[]; expect(next.find((f) => f.id === "f1")!.rangeStep).toBe(0.25); }); + + it("reverts to prior value when min input is blurred while empty", () => { + mockFormFields = [ + { + id: "f1", + label: "Rating", + parameterName: "rating", + parameterType: "number-range", + required: false, + rangeNumberType: "integer", + rangeMin: 5, + rangeMax: 10, + rangeStep: 1, + }, + ]; + render(); + const minInput = screen.getByDisplayValue("5"); + fireEvent.change(minInput, { target: { value: "" } }); + fireEvent.blur(minInput); + // Regression: previously Number("") was 0 and silently zeroed rangeMin. + expect(mockSetFormFields).not.toHaveBeenCalled(); + // Draft snaps back to prior value. + expect((minInput as HTMLInputElement).value).toBe("5"); + }); + + it("reverts to prior value on garbage input", () => { + mockFormFields = [ + { + id: "f1", + label: "Rating", + parameterName: "rating", + parameterType: "number-range", + required: false, + rangeNumberType: "integer", + rangeMin: 7, + rangeMax: 10, + rangeStep: 1, + }, + ]; + render(); + const minInput = screen.getByDisplayValue("7"); + fireEvent.change(minInput, { target: { value: "abc" } }); + fireEvent.blur(minInput); + expect(mockSetFormFields).not.toHaveBeenCalled(); + }); + + it("commits min on blur (does not commit while typing)", () => { + mockFormFields = [ + { + id: "f1", + label: "Rating", + parameterName: "rating", + parameterType: "number-range", + required: false, + rangeNumberType: "integer", + rangeMin: 0, + rangeMax: 10, + rangeStep: 1, + }, + ]; + render(); + const minInput = screen.getByDisplayValue("0"); + fireEvent.change(minInput, { target: { value: "3" } }); + // No commit yet — still typing. + expect(mockSetFormFields).not.toHaveBeenCalled(); + fireEvent.blur(minInput); + const next = mockSetFormFields.mock.calls.at(-1)![0] as FormFieldDef[]; + expect(next.find((f) => f.id === "f1")!.rangeMin).toBe(3); + }); + + it("shows inline error when min >= max", () => { + mockFormFields = [ + { + id: "f1", + label: "Rating", + parameterName: "rating", + parameterType: "number-range", + required: false, + rangeNumberType: "integer", + rangeMin: 10, + rangeMax: 5, + rangeStep: 1, + }, + ]; + render(); + expect( + screen.getByText(/Min must be less than Max/i), + ).toBeInTheDocument(); + }); + + it("shows inline error when step is 0 (forced via passthrough)", () => { + mockFormFields = [ + { + id: "f1", + label: "Rating", + parameterName: "rating", + parameterType: "number-range", + required: false, + rangeNumberType: "integer", + rangeMin: 0, + rangeMax: 10, + rangeStep: 0, + }, + ]; + render(); + expect( + screen.getByText(/Step must be greater than 0/i), + ).toBeInTheDocument(); + }); + + it("rejects a step of 0 typed by the user (keeps prior step)", () => { + mockFormFields = [ + { + id: "f1", + label: "Rating", + parameterName: "rating", + parameterType: "number-range", + required: false, + rangeNumberType: "float", + rangeMin: 0, + rangeMax: 10, + rangeStep: 0.5, + }, + ]; + render(); + const stepInput = screen.getByDisplayValue("0.5"); + fireEvent.change(stepInput, { target: { value: "0" } }); + fireEvent.blur(stepInput); + const next = mockSetFormFields.mock.calls.at(-1)?.[0] as + | FormFieldDef[] + | undefined; + if (next) { + // If commit did fire, step must have fallen back to the prior value. + expect(next.find((f) => f.id === "f1")!.rangeStep).toBe(0.5); + } + }); }); it("shows param reference hint in field content", () => { diff --git a/app/src/components/widget-editor/form-fields-editor.tsx b/app/src/components/widget-editor/form-fields-editor.tsx index 4984ea47..fd0de4c0 100644 --- a/app/src/components/widget-editor/form-fields-editor.tsx +++ b/app/src/components/widget-editor/form-fields-editor.tsx @@ -81,6 +81,141 @@ function LabeledInput({ ); } +/** + * Number-range editor: keeps min/max/step as draft strings so the user can + * clear the field while typing without it snapping to 0 (which `Number("")` + * would otherwise produce). Commits on blur and validates min0 + * with an inline error message. + */ +interface NumberRangeFieldsProps { + field: FormFieldDef; + onUpdate: (id: string, patch: Partial) => void; +} + +function NumberRangeFields({ field, onUpdate }: NumberRangeFieldsProps) { + const numType = field.rangeNumberType ?? "integer"; + const min = field.rangeMin ?? 0; + const max = field.rangeMax ?? 100; + const step = field.rangeStep ?? 1; + + // Draft strings let the user clear a field or type a partial value without + // it snapping back. We resync from props using the React "adjust state in + // render" pattern: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes + const [minDraft, setMinDraft] = React.useState(String(min)); + const [maxDraft, setMaxDraft] = React.useState(String(max)); + const [stepDraft, setStepDraft] = React.useState(String(step)); + const [prevMin, setPrevMin] = React.useState(min); + const [prevMax, setPrevMax] = React.useState(max); + const [prevStep, setPrevStep] = React.useState(step); + if (min !== prevMin) { + setPrevMin(min); + setMinDraft(String(min)); + } + if (max !== prevMax) { + setPrevMax(max); + setMaxDraft(String(max)); + } + if (step !== prevStep) { + setPrevStep(step); + setStepDraft(String(step)); + } + + const coerce = (raw: number) => + numType === "integer" ? Math.round(raw) : raw; + + const commit = (key: "rangeMin" | "rangeMax" | "rangeStep", raw: string) => { + const parsed = Number(raw); + // Empty / NaN: revert draft to prior committed value. + if (raw.trim() === "" || isNaN(parsed)) { + if (key === "rangeMin") setMinDraft(String(min)); + else if (key === "rangeMax") setMaxDraft(String(max)); + else setStepDraft(String(step)); + return; + } + if (key === "rangeStep") { + const next = + numType === "integer" ? Math.max(1, Math.round(parsed)) : parsed; + onUpdate(field.id, { rangeStep: next > 0 ? next : step }); + } else { + onUpdate(field.id, { [key]: coerce(parsed) }); + } + }; + + const validationError = + min >= max + ? "Min must be less than Max" + : step <= 0 + ? "Step must be greater than 0" + : null; + + return ( +
+
+ + +
+
+
+ + setMinDraft(e.target.value)} + onBlur={(e) => commit("rangeMin", e.target.value)} + className="h-7 text-xs" + /> +
+
+ + setMaxDraft(e.target.value)} + onBlur={(e) => commit("rangeMax", e.target.value)} + className="h-7 text-xs" + /> +
+
+ + setStepDraft(e.target.value)} + onBlur={(e) => commit("rangeStep", e.target.value)} + className="h-7 text-xs" + /> +
+
+ {validationError && ( +

+ {validationError} +

+ )} +
+ ); +} + interface SortableFieldItemProps { field: FormFieldDef; index: number; @@ -255,73 +390,7 @@ function SortableFieldItem({ {/* Range config (for number-range) */} {field.parameterType === "number-range" && ( -
-
- - -
-
- { - const raw = Number(v); - const numType = field.rangeNumberType ?? "integer"; - onUpdate(field.id, { - rangeMin: numType === "integer" ? Math.round(raw) : raw, - }); - }} - /> - { - const raw = Number(v); - const numType = field.rangeNumberType ?? "integer"; - onUpdate(field.id, { - rangeMax: numType === "integer" ? Math.round(raw) : raw, - }); - }} - /> - { - const raw = Number(v); - const numType = field.rangeNumberType ?? "integer"; - onUpdate(field.id, { - rangeStep: - numType === "integer" - ? Math.max(1, Math.round(raw)) - : raw, - }); - }} - /> -
-
+ )} {/* Placeholder (for text/select types) */} diff --git a/component/src/components/composed/__tests__/parameter-widgets.test.tsx b/component/src/components/composed/__tests__/parameter-widgets.test.tsx index 87e3b2a1..a36e3fb5 100644 --- a/component/src/components/composed/__tests__/parameter-widgets.test.tsx +++ b/component/src/components/composed/__tests__/parameter-widgets.test.tsx @@ -17,14 +17,18 @@ import { describe("TextInputParameter", () => { it("renders with a label matching parameterName", () => { render( - + , ); expect(screen.getByText("city")).toBeInTheDocument(); }); it("renders the current value in the input", () => { render( - + , ); const input = screen.getByRole("textbox"); expect(input).toHaveValue("Berlin"); @@ -33,22 +37,30 @@ describe("TextInputParameter", () => { it("calls onChange when the user types", () => { const onChange = vi.fn(); render( - + , ); - fireEvent.change(screen.getByRole("textbox"), { target: { value: "Paris" } }); + fireEvent.change(screen.getByRole("textbox"), { + target: { value: "Paris" }, + }); expect(onChange).toHaveBeenCalledWith("Paris"); }); it("shows a clear button when value is set", () => { render( - + , ); - expect(screen.getByRole("button", { name: /clear city/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /clear city/i }), + ).toBeInTheDocument(); }); it("hides the clear button when value is empty", () => { render( - + , ); expect(screen.queryByRole("button", { name: /clear/i })).toBeNull(); }); @@ -56,7 +68,11 @@ describe("TextInputParameter", () => { it("calls onChange with empty string when clear button is clicked", () => { const onChange = vi.fn(); render( - + , ); fireEvent.click(screen.getByRole("button", { name: /clear/i })); expect(onChange).toHaveBeenCalledWith(""); @@ -69,14 +85,19 @@ describe("TextInputParameter", () => { value="" onChange={vi.fn()} placeholder="Type something…" - /> + />, ); expect(screen.getByPlaceholderText("Type something…")).toBeInTheDocument(); }); it("applies extra className to the root element", () => { const { container } = render( - + , ); expect(container.firstChild).toHaveClass("extra-cls"); }); @@ -97,7 +118,7 @@ describe("ParamSelector", () => { options={options} value="" onChange={vi.fn()} - /> + />, ); expect(screen.getByText("dbType")).toBeInTheDocument(); }); @@ -110,10 +131,12 @@ describe("ParamSelector", () => { value="" onChange={vi.fn()} loading - /> + />, ); // Loading state renders skeleton elements (animate-pulse class from Skeleton component) - expect(container.querySelectorAll('[class*="animate-pulse"]').length).toBeGreaterThan(0); + expect( + container.querySelectorAll('[class*="animate-pulse"]').length, + ).toBeGreaterThan(0); // The select trigger should not be present during loading expect(screen.queryByRole("combobox")).toBeNull(); }); @@ -125,9 +148,11 @@ describe("ParamSelector", () => { options={options} value="neo4j" onChange={vi.fn()} - /> + />, ); - expect(screen.getByRole("button", { name: /clear dbType/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /clear dbType/i }), + ).toBeInTheDocument(); }); it("calls onChange with empty string when clear is clicked", () => { @@ -138,7 +163,7 @@ describe("ParamSelector", () => { options={options} value="neo4j" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: /clear/i })); expect(onChange).toHaveBeenCalledWith(""); @@ -151,7 +176,7 @@ describe("ParamSelector", () => { options={options} value="" onChange={vi.fn()} - /> + />, ); expect(screen.queryByRole("button", { name: /clear/i })).toBeNull(); }); @@ -163,7 +188,7 @@ describe("ParamSelector", () => { options={options} value="" onChange={vi.fn()} - /> + />, ); expect(screen.getByRole("combobox")).toBeInTheDocument(); }); @@ -176,7 +201,7 @@ describe("ParamSelector", () => { value="" onChange={vi.fn()} className="my-class" - /> + />, ); expect(container.firstChild).toHaveClass("my-class"); }); @@ -199,7 +224,7 @@ describe("ParamMultiSelector", () => { options={options} values={[]} onChange={vi.fn()} - /> + />, ); expect(screen.getByText("tags")).toBeInTheDocument(); }); @@ -212,7 +237,7 @@ describe("ParamMultiSelector", () => { values={[]} onChange={vi.fn()} placeholder="Pick tags…" - /> + />, ); expect(screen.getByText("Pick tags…")).toBeInTheDocument(); }); @@ -224,7 +249,7 @@ describe("ParamMultiSelector", () => { options={options} values={["a", "b"]} onChange={vi.fn()} - /> + />, ); expect(screen.getByText("Alpha")).toBeInTheDocument(); expect(screen.getByText("Beta")).toBeInTheDocument(); @@ -237,7 +262,7 @@ describe("ParamMultiSelector", () => { options={options} values={["a"]} onChange={vi.fn()} - /> + />, ); expect(screen.getByRole("button", { name: /clear/i })).toBeInTheDocument(); }); @@ -250,7 +275,7 @@ describe("ParamMultiSelector", () => { options={options} values={["a", "b"]} onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: /clear/i })); expect(onChange).toHaveBeenCalledWith([]); @@ -263,7 +288,7 @@ describe("ParamMultiSelector", () => { options={options} values={[]} onChange={vi.fn()} - /> + />, ); expect(screen.queryByRole("button", { name: /^clear$/i })).toBeNull(); }); @@ -276,7 +301,7 @@ describe("ParamMultiSelector", () => { values={[]} onChange={vi.fn()} loading - /> + />, ); // When loading, the combobox trigger should not be present expect(screen.queryByRole("combobox")).toBeNull(); @@ -290,7 +315,7 @@ describe("ParamMultiSelector", () => { values={["a", "b", "c", "d"]} onChange={vi.fn()} maxDisplay={2} - /> + />, ); // +2 overflow badge for items c and d expect(screen.getByText("+2")).toBeInTheDocument(); @@ -304,7 +329,7 @@ describe("ParamMultiSelector", () => { options={options} values={["a", "b"]} onChange={onChange} - /> + />, ); // Click the close button on the first badge (Alpha) const alphaClose = document.querySelector('button[type="button"].ml-1'); @@ -322,7 +347,7 @@ describe("ParamMultiSelector", () => { values={[]} onChange={vi.fn()} className="custom-multi" - /> + />, ); expect(container.firstChild).toHaveClass("custom-multi"); }); @@ -337,7 +362,7 @@ describe("DatePickerParameter", () => { parameterName="eventDate" value="" onChange={vi.fn()} - /> + />, ); expect(screen.getByText("eventDate")).toBeInTheDocument(); }); @@ -348,7 +373,7 @@ describe("DatePickerParameter", () => { parameterName="eventDate" value="" onChange={vi.fn()} - /> + />, ); expect(screen.getByText(/pick a date/i)).toBeInTheDocument(); }); @@ -359,7 +384,7 @@ describe("DatePickerParameter", () => { parameterName="eventDate" value="2024-06-15" onChange={vi.fn()} - /> + />, ); expect(screen.getByText(/jun 15, 2024/i)).toBeInTheDocument(); }); @@ -370,9 +395,11 @@ describe("DatePickerParameter", () => { parameterName="eventDate" value="2024-06-15" onChange={vi.fn()} - /> + />, ); - expect(screen.getByRole("button", { name: /clear eventDate/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /clear eventDate/i }), + ).toBeInTheDocument(); }); it("calls onChange with empty string when clear is clicked", () => { @@ -382,7 +409,7 @@ describe("DatePickerParameter", () => { parameterName="eventDate" value="2024-06-15" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: /clear/i })); expect(onChange).toHaveBeenCalledWith(""); @@ -394,7 +421,7 @@ describe("DatePickerParameter", () => { parameterName="eventDate" value="" onChange={vi.fn()} - /> + />, ); expect(screen.queryByRole("button", { name: /clear/i })).toBeNull(); }); @@ -406,7 +433,7 @@ describe("DatePickerParameter", () => { value="" onChange={vi.fn()} className="date-cls" - /> + />, ); expect(container.firstChild).toHaveClass("date-cls"); }); @@ -417,7 +444,7 @@ describe("DatePickerParameter", () => { parameterName="eventDate" value="" onChange={vi.fn()} - /> + />, ); // The Radix Popover trigger is labeled by aria-labelledby → "eventDate" label const triggerBtn = screen.getByRole("button", { name: "eventDate" }); @@ -433,7 +460,7 @@ describe("DatePickerParameter", () => { parameterName="eventDate" value="" onChange={onChange} - /> + />, ); // Open the calendar fireEvent.click(screen.getByRole("button", { name: "eventDate" })); @@ -445,7 +472,9 @@ describe("DatePickerParameter", () => { const dayBtn = document.querySelector("button[data-day]"); if (dayBtn) { fireEvent.click(dayBtn); - expect(onChange).toHaveBeenCalledWith(expect.stringMatching(/^\d{4}-\d{2}-\d{2}$/)); + expect(onChange).toHaveBeenCalledWith( + expect.stringMatching(/^\d{4}-\d{2}-\d{2}$/), + ); } }); }); @@ -460,7 +489,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={vi.fn()} - /> + />, ); expect(screen.getByText("period")).toBeInTheDocument(); }); @@ -472,7 +501,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={vi.fn()} - /> + />, ); expect(screen.getByText(/pick a date range/i)).toBeInTheDocument(); }); @@ -484,7 +513,7 @@ describe("DateRangeParameter", () => { from="2024-06-01" to="" onChange={vi.fn()} - /> + />, ); expect(screen.getByText(/jun 1, 2024/i)).toBeInTheDocument(); }); @@ -496,7 +525,7 @@ describe("DateRangeParameter", () => { from="2024-06-01" to="2024-06-30" onChange={vi.fn()} - /> + />, ); expect(screen.getByText(/jun 1, 2024/i)).toBeInTheDocument(); expect(screen.getByText(/jun 30, 2024/i)).toBeInTheDocument(); @@ -509,9 +538,11 @@ describe("DateRangeParameter", () => { from="2024-06-01" to="" onChange={vi.fn()} - /> + />, ); - expect(screen.getByRole("button", { name: /clear period/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /clear period/i }), + ).toBeInTheDocument(); }); it("shows clear button when to is set", () => { @@ -521,9 +552,11 @@ describe("DateRangeParameter", () => { from="" to="2024-06-30" onChange={vi.fn()} - /> + />, ); - expect(screen.getByRole("button", { name: /clear period/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /clear period/i }), + ).toBeInTheDocument(); }); it("calls onChange with empty strings when clear is clicked", () => { @@ -534,7 +567,7 @@ describe("DateRangeParameter", () => { from="2024-06-01" to="2024-06-30" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: /clear period/i })); expect(onChange).toHaveBeenCalledWith("", ""); @@ -547,7 +580,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={vi.fn()} - /> + />, ); expect(screen.queryByRole("button", { name: /clear/i })).toBeNull(); }); @@ -560,7 +593,7 @@ describe("DateRangeParameter", () => { to="" onChange={vi.fn()} className="range-cls" - /> + />, ); expect(container.firstChild).toHaveClass("range-cls"); }); @@ -572,7 +605,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={vi.fn()} - /> + />, ); // Radix Popover trigger is labeled by aria-labelledby → "period" label const triggerBtn = screen.getByRole("button", { name: "period" }); @@ -594,7 +627,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: "period" })); fireEvent.click(screen.getByText("Today")); @@ -612,7 +645,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: "period" })); fireEvent.click(screen.getByText("Last 7 days")); @@ -630,7 +663,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: "period" })); fireEvent.click(screen.getByText("Last 30 days")); @@ -648,7 +681,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: "period" })); fireEvent.click(screen.getByText("This month")); @@ -666,7 +699,7 @@ describe("DateRangeParameter", () => { from="" to="" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: "period" })); fireEvent.click(screen.getByText("This year")); @@ -680,23 +713,29 @@ describe("DateRangeParameter", () => { describe("DateRelativePicker", () => { it("renders the parameter label", () => { render( - + , ); expect(screen.getByText("window")).toBeInTheDocument(); }); it("renders all preset buttons", () => { render( - + , ); for (const preset of RELATIVE_DATE_PRESETS) { - expect(screen.getByRole("button", { name: preset.label })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: preset.label }), + ).toBeInTheDocument(); } }); it("marks the active preset button as pressed", () => { render( - + , ); const btn = screen.getByRole("button", { name: "Last 7 days" }); expect(btn).toHaveAttribute("aria-pressed", "true"); @@ -704,7 +743,11 @@ describe("DateRelativePicker", () => { it("marks inactive preset buttons as not pressed", () => { render( - + , ); const btn = screen.getByRole("button", { name: "Last 7 days" }); expect(btn).toHaveAttribute("aria-pressed", "false"); @@ -713,7 +756,11 @@ describe("DateRelativePicker", () => { it("calls onChange with the preset key when a button is clicked", () => { const onChange = vi.fn(); render( - + , ); fireEvent.click(screen.getByRole("button", { name: "Today" })); expect(onChange).toHaveBeenCalledWith("today"); @@ -722,7 +769,11 @@ describe("DateRelativePicker", () => { it("calls onChange with empty string when the active preset is clicked again (toggle off)", () => { const onChange = vi.fn(); render( - + , ); fireEvent.click(screen.getByRole("button", { name: "Today" })); expect(onChange).toHaveBeenCalledWith(""); @@ -733,7 +784,11 @@ describe("DateRelativePicker", () => { for (const preset of RELATIVE_DATE_PRESETS) { onChange.mockClear(); const { unmount } = render( - + , ); fireEvent.click(screen.getByRole("button", { name: preset.label })); expect(onChange).toHaveBeenCalledWith(preset.key); @@ -743,7 +798,7 @@ describe("DateRelativePicker", () => { it("renders a group role for accessibility", () => { render( - + , ); expect(screen.getByRole("group")).toBeInTheDocument(); }); @@ -755,7 +810,7 @@ describe("DateRelativePicker", () => { value="" onChange={vi.fn()} className="rel-cls" - /> + />, ); expect(container.firstChild).toHaveClass("rel-cls"); }); @@ -773,7 +828,7 @@ describe("NumberRangeSlider", () => { value={null} onChange={vi.fn()} onClear={vi.fn()} - /> + />, ); expect(screen.getByText("price")).toBeInTheDocument(); }); @@ -787,7 +842,7 @@ describe("NumberRangeSlider", () => { value={null} onChange={vi.fn()} onClear={vi.fn()} - /> + />, ); expect(screen.getByText("10")).toBeInTheDocument(); expect(screen.getByText("999")).toBeInTheDocument(); @@ -802,9 +857,11 @@ describe("NumberRangeSlider", () => { value={[100, 500]} onChange={vi.fn()} onClear={vi.fn()} - /> + />, ); - expect(screen.getByRole("button", { name: /clear price/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /clear price/i }), + ).toBeInTheDocument(); }); it("hides Reset button when value is null", () => { @@ -816,7 +873,7 @@ describe("NumberRangeSlider", () => { value={null} onChange={vi.fn()} onClear={vi.fn()} - /> + />, ); expect(screen.queryByRole("button", { name: /clear price/i })).toBeNull(); }); @@ -831,7 +888,7 @@ describe("NumberRangeSlider", () => { value={[100, 500]} onChange={vi.fn()} onClear={onClear} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: /clear price/i })); expect(onClear).toHaveBeenCalled(); @@ -847,15 +904,15 @@ describe("NumberRangeSlider", () => { onChange={vi.fn()} onClear={vi.fn()} showInputs - /> + />, ); - const minInput = screen.getByRole("spinbutton", { name: /price minimum/i }); - const maxInput = screen.getByRole("spinbutton", { name: /price maximum/i }); - expect(minInput).toHaveValue(150); - expect(maxInput).toHaveValue(750); + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); + const maxInput = screen.getByRole("textbox", { name: /price maximum/i }); + expect(minInput).toHaveValue("150"); + expect(maxInput).toHaveValue("750"); }); - it("calls onChange when min input changes", () => { + it("calls onChange when min input is committed on blur", () => { const onChange = vi.fn(); render( { onChange={onChange} onClear={vi.fn()} showInputs - /> + />, ); - const minInput = screen.getByRole("spinbutton", { name: /price minimum/i }); + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); fireEvent.change(minInput, { target: { value: "200" } }); + fireEvent.blur(minInput); expect(onChange).toHaveBeenCalledWith([200, 1000]); }); - it("calls onChange when max input changes", () => { + it("calls onChange when max input is committed on blur", () => { const onChange = vi.fn(); render( { onChange={onChange} onClear={vi.fn()} showInputs - /> + />, ); - const maxInput = screen.getByRole("spinbutton", { name: /price maximum/i }); + const maxInput = screen.getByRole("textbox", { name: /price maximum/i }); fireEvent.change(maxInput, { target: { value: "800" } }); + fireEvent.blur(maxInput); expect(onChange).toHaveBeenCalledWith([0, 800]); }); + it("does not call onChange while user is still typing", () => { + const onChange = vi.fn(); + render( + , + ); + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); + fireEvent.change(minInput, { target: { value: "2" } }); + fireEvent.change(minInput, { target: { value: "20" } }); + expect(onChange).not.toHaveBeenCalled(); + }); + + it("allows typing a leading minus without snapping (regression: NaN no-op)", () => { + const onChange = vi.fn(); + render( + , + ); + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); + fireEvent.change(minInput, { target: { value: "-" } }); + // The draft accepts "-" without committing onChange or NaN. + expect(minInput).toHaveValue("-"); + expect(onChange).not.toHaveBeenCalled(); + fireEvent.change(minInput, { target: { value: "-30" } }); + fireEvent.blur(minInput); + expect(onChange).toHaveBeenCalledWith([-30, 50]); + }); + + it("reverts to prior value when user blurs an empty draft", () => { + const onChange = vi.fn(); + render( + , + ); + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); + fireEvent.change(minInput, { target: { value: "" } }); + fireEvent.blur(minInput); + expect(onChange).not.toHaveBeenCalled(); + expect(minInput).toHaveValue("100"); + }); + + it("commits on Enter key", () => { + const onChange = vi.fn(); + render( + , + ); + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); + fireEvent.change(minInput, { target: { value: "250" } }); + fireEvent.keyDown(minInput, { key: "Enter" }); + expect(onChange).toHaveBeenCalledWith([250, 1000]); + }); + it("clamps min input to no more than current max", () => { const onChange = vi.fn(); render( @@ -902,11 +1042,11 @@ describe("NumberRangeSlider", () => { onChange={onChange} onClear={vi.fn()} showInputs - /> + />, ); - const minInput = screen.getByRole("spinbutton", { name: /price minimum/i }); - // Try to set min above current max of 500 + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); fireEvent.change(minInput, { target: { value: "600" } }); + fireEvent.blur(minInput); expect(onChange).toHaveBeenCalledWith([500, 500]); }); @@ -921,11 +1061,11 @@ describe("NumberRangeSlider", () => { onChange={onChange} onClear={vi.fn()} showInputs - /> + />, ); - const maxInput = screen.getByRole("spinbutton", { name: /price maximum/i }); - // Try to set max below current min of 200 + const maxInput = screen.getByRole("textbox", { name: /price maximum/i }); fireEvent.change(maxInput, { target: { value: "100" } }); + fireEvent.blur(maxInput); expect(onChange).toHaveBeenCalledWith([200, 200]); }); @@ -940,11 +1080,11 @@ describe("NumberRangeSlider", () => { onChange={onChange} onClear={vi.fn()} showInputs - /> + />, ); - const minInput = screen.getByRole("spinbutton", { name: /price minimum/i }); - // Setting min to below the overall min (50) should clamp to 50 + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); fireEvent.change(minInput, { target: { value: "10" } }); + fireEvent.blur(minInput); expect(onChange).toHaveBeenCalledWith([50, 500]); }); @@ -959,11 +1099,11 @@ describe("NumberRangeSlider", () => { onChange={onChange} onClear={vi.fn()} showInputs - /> + />, ); - const maxInput = screen.getByRole("spinbutton", { name: /price maximum/i }); - // Setting max above the overall max (800) should clamp to 800 + const maxInput = screen.getByRole("textbox", { name: /price maximum/i }); fireEvent.change(maxInput, { target: { value: "1200" } }); + fireEvent.blur(maxInput); expect(onChange).toHaveBeenCalledWith([100, 800]); }); @@ -977,9 +1117,9 @@ describe("NumberRangeSlider", () => { onChange={vi.fn()} onClear={vi.fn()} showInputs={false} - /> + />, ); - expect(screen.queryByRole("spinbutton")).toBeNull(); + expect(screen.queryByRole("textbox")).toBeNull(); }); it("uses min/max as defaults when value is null", () => { @@ -992,10 +1132,14 @@ describe("NumberRangeSlider", () => { onChange={vi.fn()} onClear={vi.fn()} showInputs - /> + />, + ); + expect(screen.getByRole("textbox", { name: /qty minimum/i })).toHaveValue( + "5", + ); + expect(screen.getByRole("textbox", { name: /qty maximum/i })).toHaveValue( + "50", ); - expect(screen.getByRole("spinbutton", { name: /qty minimum/i })).toHaveValue(5); - expect(screen.getByRole("spinbutton", { name: /qty maximum/i })).toHaveValue(50); }); it("applies className to root element", () => { @@ -1008,7 +1152,7 @@ describe("NumberRangeSlider", () => { onChange={vi.fn()} onClear={vi.fn()} className="slider-cls" - /> + />, ); expect(container.firstChild).toHaveClass("slider-cls"); }); @@ -1029,7 +1173,7 @@ describe("CascadingSelector", () => { options={options} value="" onChange={vi.fn()} - /> + />, ); expect(screen.getByText("subCategory")).toBeInTheDocument(); }); @@ -1042,7 +1186,7 @@ describe("CascadingSelector", () => { value="" onChange={vi.fn()} parentParameterName="category" - /> + />, ); expect(screen.getByText(/depends on category/i)).toBeInTheDocument(); }); @@ -1056,7 +1200,7 @@ describe("CascadingSelector", () => { onChange={vi.fn()} parentParameterName="category" parentValue="" - /> + />, ); // The select trigger should be disabled const trigger = screen.getByRole("combobox"); @@ -1072,7 +1216,7 @@ describe("CascadingSelector", () => { onChange={vi.fn()} parentParameterName="category" parentValue="cat1" - /> + />, ); const trigger = screen.getByRole("combobox"); expect(trigger).not.toBeDisabled(); @@ -1085,9 +1229,11 @@ describe("CascadingSelector", () => { options={options} value="sub1" onChange={vi.fn()} - /> + />, ); - expect(screen.getByRole("button", { name: /clear subCategory/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /clear subCategory/i }), + ).toBeInTheDocument(); }); it("calls onChange with empty string when clear is clicked", () => { @@ -1098,7 +1244,7 @@ describe("CascadingSelector", () => { options={options} value="sub1" onChange={onChange} - /> + />, ); fireEvent.click(screen.getByRole("button", { name: /clear/i })); expect(onChange).toHaveBeenCalledWith(""); @@ -1112,12 +1258,14 @@ describe("CascadingSelector", () => { value="" onChange={vi.fn()} loading - /> + />, ); // The select combobox should not be present during loading expect(screen.queryByRole("combobox")).toBeNull(); // Skeleton placeholders should be rendered (animate-pulse divs) - expect(container.querySelectorAll(".animate-pulse").length).toBeGreaterThanOrEqual(2); + expect( + container.querySelectorAll(".animate-pulse").length, + ).toBeGreaterThanOrEqual(2); }); it("does not show dependency hint when parentParameterName is not provided", () => { @@ -1127,7 +1275,7 @@ describe("CascadingSelector", () => { options={options} value="" onChange={vi.fn()} - /> + />, ); expect(screen.queryByText(/depends on/i)).toBeNull(); }); @@ -1141,7 +1289,7 @@ describe("CascadingSelector", () => { onChange={vi.fn()} parentParameterName="category" parentValue="" - /> + />, ); // The placeholder includes the parent name expect(screen.getByText(/select category first/i)).toBeInTheDocument(); @@ -1155,7 +1303,7 @@ describe("CascadingSelector", () => { value="" onChange={vi.fn()} placeholder="Choose sub-category…" - /> + />, ); expect(screen.getByText("Choose sub-category…")).toBeInTheDocument(); }); @@ -1167,7 +1315,7 @@ describe("CascadingSelector", () => { options={options} value="" onChange={vi.fn()} - /> + />, ); expect(screen.queryByRole("button", { name: /clear/i })).toBeNull(); }); @@ -1180,7 +1328,7 @@ describe("CascadingSelector", () => { value="" onChange={vi.fn()} className="cascade-cls" - /> + />, ); expect(container.firstChild).toHaveClass("cascade-cls"); }); @@ -1192,7 +1340,7 @@ describe("CascadingSelector", () => { options={options} value="" onChange={vi.fn()} - /> + />, ); const trigger = screen.getByRole("combobox"); expect(trigger).not.toBeDisabled(); diff --git a/component/src/components/composed/parameter-widgets/number-range-slider.tsx b/component/src/components/composed/parameter-widgets/number-range-slider.tsx index 09b5a6d0..56db3fa2 100644 --- a/component/src/components/composed/parameter-widgets/number-range-slider.tsx +++ b/component/src/components/composed/parameter-widgets/number-range-slider.tsx @@ -1,5 +1,6 @@ "use client"; +import { useState } from "react"; import { X } from "lucide-react"; import { Label } from "@/components/ui/label"; import { Button } from "@/components/ui/button"; @@ -45,20 +46,55 @@ function NumberRangeSlider({ const coerce = (n: number) => (numberType === "integer" ? Math.round(n) : n); + // Draft strings let the user type "-" or "" or partial numbers without the + // input value snapping back. We resync from `current` whenever it changes + // externally (e.g. slider drag, clear) using the React "adjust state in + // render" pattern: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes + const [minDraft, setMinDraft] = useState(String(current[0])); + const [maxDraft, setMaxDraft] = useState(String(current[1])); + const [prevMin, setPrevMin] = useState(current[0]); + const [prevMax, setPrevMax] = useState(current[1]); + if (current[0] !== prevMin) { + setPrevMin(current[0]); + setMinDraft(String(current[0])); + } + if (current[1] !== prevMax) { + setPrevMax(current[1]); + setMaxDraft(String(current[1])); + } + const handleSliderChange = (vals: number[]) => { - onChange([coerce(vals[0]), coerce(vals[1])]); + // Radix can emit a 1-element array when min===max; fall back to the + // other handle so we always emit a [number, number] tuple. + const next0 = vals[0] ?? current[0]; + const next1 = vals[1] ?? next0; + onChange([coerce(next0), coerce(next1)]); }; - const handleMinInput = (raw: string) => { + const commitMin = (raw: string) => { + if (raw === "" || raw === "-") { + setMinDraft(String(current[0])); + return; + } const num = Number(raw); - if (isNaN(num)) return; + if (isNaN(num)) { + setMinDraft(String(current[0])); + return; + } const clamped = Math.min(Math.max(num, min), current[1]); onChange([coerce(clamped), current[1]]); }; - const handleMaxInput = (raw: string) => { + const commitMax = (raw: string) => { + if (raw === "" || raw === "-") { + setMaxDraft(String(current[1])); + return; + } const num = Number(raw); - if (isNaN(num)) return; + if (isNaN(num)) { + setMaxDraft(String(current[1])); + return; + } const clamped = Math.max(Math.min(num, max), current[0]); onChange([current[0], coerce(clamped)]); }; @@ -90,12 +126,14 @@ function NumberRangeSlider({ {showInputs && (
handleMinInput(e.target.value)} - min={min} - max={current[1]} - step={step} + type="text" + inputMode="numeric" + value={minDraft} + onChange={(e) => setMinDraft(e.target.value)} + onBlur={(e) => commitMin(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") commitMin(e.currentTarget.value); + }} className="w-20 text-center text-sm h-7" aria-label={`${parameterName} minimum`} /> @@ -103,12 +141,14 @@ function NumberRangeSlider({ to handleMaxInput(e.target.value)} - min={current[0]} - max={max} - step={step} + type="text" + inputMode="numeric" + value={maxDraft} + onChange={(e) => setMaxDraft(e.target.value)} + onBlur={(e) => commitMax(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") commitMax(e.currentTarget.value); + }} className="w-20 text-center text-sm h-7" aria-label={`${parameterName} maximum`} /> From feac9475221ee0205587de8e66c0a4d3b548fe5d Mon Sep 17 00:00:00 2001 From: alfredorubin96 Date: Tue, 19 May 2026 02:48:58 +0200 Subject: [PATCH 2/3] test(e2e): adapt number-range E2E to draft inputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NumberRangeSlider now uses text inputs (inputMode=numeric) with commit-on-blur, so: - accessible role is `textbox`, not `spinbutton` — target inputs by their aria-label (` minimum` / ` maximum`) - `fill()` only updates the draft; commit requires blur — add an explicit `press("Tab")` after each fill so the parameter store actually updates Co-Authored-By: Claude Opus 4 --- app/e2e/parameter-types.spec.ts | 19 ++++++++++++------- app/e2e/parameters.spec.ts | 4 ++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/e2e/parameter-types.spec.ts b/app/e2e/parameter-types.spec.ts index 70d92618..813d3653 100644 --- a/app/e2e/parameter-types.spec.ts +++ b/app/e2e/parameter-types.spec.ts @@ -182,13 +182,18 @@ test.describe("Parameter widget types", () => { // NumberRangeSlider exposes its inputs via `showInputs` — fill them // directly to drive deterministic values (avoids drag math on the - // slider handles). - const inputs = page.getByRole("spinbutton"); - await inputs.first().waitFor({ state: "visible", timeout: 15_000 }); - await inputs.first().fill("1999"); - await inputs.first().press("Tab"); - await inputs.last().fill("2003"); - await inputs.last().press("Tab"); + // slider handles). Inputs are `type="text" inputMode="numeric"` so + // the user can type partial values ("-", "") without snapping; + // accessible role is `textbox` (not `spinbutton`). We target by the + // aria-label set by NumberRangeSlider to avoid grabbing unrelated + // textboxes on the page. + const minInput = page.getByLabel("yr minimum"); + const maxInput = page.getByLabel("yr maximum"); + await minInput.waitFor({ state: "visible", timeout: 15_000 }); + await minInput.fill("1999"); + await minInput.press("Tab"); + await maxInput.fill("2003"); + await maxInput.press("Tab"); // count(m) result renders somewhere on the card; assert a positive // integer (movies DB always has at least one movie in 1999–2003). diff --git a/app/e2e/parameters.spec.ts b/app/e2e/parameters.spec.ts index a9af92bb..d075efa5 100644 --- a/app/e2e/parameters.spec.ts +++ b/app/e2e/parameters.spec.ts @@ -1386,14 +1386,18 @@ test.describe("Number-range parameter widget", () => { await expect(maxInput).toHaveValue("2020"); // Change the min input — should trigger parameter set and show Reset button. + // Inputs use draft state and only commit on blur/Enter (so the user can + // type "-" or partial numbers without snapping); Tab triggers blur. // Two "Reset" buttons may appear (slider + parameter bar), so use .first(). await minInput.fill("1950"); + await minInput.press("Tab"); await expect( page.getByRole("button", { name: "Reset" }).first(), ).toBeVisible({ timeout: 5_000 }); // Change the max input await maxInput.fill("2000"); + await maxInput.press("Tab"); await expect(maxInput).toHaveValue("2000"); // Click Reset — should clear the range (both slider and parameter bar Reset disappear) From 5ff258fdc3455b03a60b24ec3e181a2c51a0284b Mon Sep 17 00:00:00 2001 From: alfredorubin96 Date: Tue, 19 May 2026 03:46:18 +0200 Subject: [PATCH 3/3] test(number-range): cover float coerce, NaN revert, draft resync (#857) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brings Sonar new-code coverage for number-range-slider.tsx above the 80% gate. Targets previously uncovered branches: - float numberType (preserves decimals; no Math.round) - integer numberType (rounds 3.7 → 4) - NaN draft revert on blur for both min and max inputs - Enter key commits on the max input (already covered for min) - draft inputs resync when the value prop changes externally (e.g. slider drag while inputs are unfocused) - showInputs defaults to true when prop is omitted Co-Authored-By: Claude Opus 4 --- .../__tests__/parameter-widgets.test.tsx | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/component/src/components/composed/__tests__/parameter-widgets.test.tsx b/component/src/components/composed/__tests__/parameter-widgets.test.tsx index a36e3fb5..d542feae 100644 --- a/component/src/components/composed/__tests__/parameter-widgets.test.tsx +++ b/component/src/components/composed/__tests__/parameter-widgets.test.tsx @@ -1156,6 +1156,170 @@ describe("NumberRangeSlider", () => { ); expect(container.firstChild).toHaveClass("slider-cls"); }); + + // ── Coverage-uplift cases (regression: #857) ─────────────────────── + // + // The branches below previously had no direct test: + // - float coerce (no Math.round) + // - NaN draft revert on blur + // - Enter key on max input + // - draft resync when value prop changes externally (slider drag) + // - showInputs default (true) + + it("preserves decimals when numberType is float", () => { + const onChange = vi.fn(); + render( + , + ); + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); + fireEvent.change(minInput, { target: { value: "2.5" } }); + fireEvent.blur(minInput); + expect(onChange).toHaveBeenCalledWith([2.5, 10]); + }); + + it("rounds to nearest integer by default", () => { + const onChange = vi.fn(); + render( + , + ); + const minInput = screen.getByRole("textbox", { name: /qty minimum/i }); + fireEvent.change(minInput, { target: { value: "3.7" } }); + fireEvent.blur(minInput); + expect(onChange).toHaveBeenCalledWith([4, 10]); + }); + + it("reverts to prior value on blur when min draft is NaN", () => { + const onChange = vi.fn(); + render( + , + ); + const minInput = screen.getByRole("textbox", { name: /price minimum/i }); + fireEvent.change(minInput, { target: { value: "abc" } }); + fireEvent.blur(minInput); + expect(onChange).not.toHaveBeenCalled(); + expect(minInput).toHaveValue("10"); + }); + + it("reverts to prior value on blur when max draft is NaN or empty", () => { + const onChange = vi.fn(); + render( + , + ); + const maxInput = screen.getByRole("textbox", { name: /price maximum/i }); + fireEvent.change(maxInput, { target: { value: "" } }); + fireEvent.blur(maxInput); + expect(onChange).not.toHaveBeenCalled(); + expect(maxInput).toHaveValue("50"); + + fireEvent.change(maxInput, { target: { value: "xyz" } }); + fireEvent.blur(maxInput); + expect(onChange).not.toHaveBeenCalled(); + }); + + it("commits max on Enter key", () => { + const onChange = vi.fn(); + render( + , + ); + const maxInput = screen.getByRole("textbox", { name: /price maximum/i }); + fireEvent.change(maxInput, { target: { value: "750" } }); + fireEvent.keyDown(maxInput, { key: "Enter" }); + expect(onChange).toHaveBeenCalledWith([0, 750]); + }); + + it("re-syncs draft inputs when the value prop changes externally", () => { + // Simulates a slider drag committing a new value while the user + // hasn't focused the input — drafts must follow the source of truth. + const { rerender } = render( + , + ); + expect(screen.getByRole("textbox", { name: /price minimum/i })).toHaveValue( + "10", + ); + + rerender( + , + ); + expect(screen.getByRole("textbox", { name: /price minimum/i })).toHaveValue( + "25", + ); + expect(screen.getByRole("textbox", { name: /price maximum/i })).toHaveValue( + "75", + ); + }); + + it("defaults showInputs to true when prop is omitted", () => { + render( + , + ); + expect( + screen.getByRole("textbox", { name: /price minimum/i }), + ).toBeInTheDocument(); + }); }); // ─── CascadingSelector ────────────────────────────────────────────────────────