Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions app/e2e/parameter-types.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
4 changes: 4 additions & 0 deletions app/e2e/parameters.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
55 changes: 34 additions & 21 deletions app/src/components/parameters/__tests__/param-number-range.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
});
});
13 changes: 10 additions & 3 deletions app/src/components/parameters/param-number-range.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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] =
Expand Down
142 changes: 142 additions & 0 deletions app/src/components/widget-editor/__tests__/form-fields-editor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ describe("FormFieldsEditor", () => {
render(<FormFieldsEditor />);
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")!;
Expand All @@ -380,6 +381,7 @@ describe("FormFieldsEditor", () => {
render(<FormFieldsEditor />);
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")!;
Expand Down Expand Up @@ -426,6 +428,7 @@ describe("FormFieldsEditor", () => {
render(<FormFieldsEditor />);
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);
});
Expand All @@ -447,6 +450,7 @@ describe("FormFieldsEditor", () => {
render(<FormFieldsEditor />);
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);
});
Expand All @@ -468,6 +472,7 @@ describe("FormFieldsEditor", () => {
render(<FormFieldsEditor />);
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);
Expand All @@ -490,9 +495,146 @@ describe("FormFieldsEditor", () => {
render(<FormFieldsEditor />);
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(<FormFieldsEditor />);
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(<FormFieldsEditor />);
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(<FormFieldsEditor />);
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(<FormFieldsEditor />);
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(<FormFieldsEditor />);
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(<FormFieldsEditor />);
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", () => {
Expand Down
Loading
Loading