diff --git a/app/src/components/widget-editor-modal.tsx b/app/src/components/widget-editor-modal.tsx index d097701d..bffc894a 100644 --- a/app/src/components/widget-editor-modal.tsx +++ b/app/src/components/widget-editor-modal.tsx @@ -654,7 +654,10 @@ export function WidgetEditorModal({ connections={connections} showConnection={ !isContentOnly && - (isForm || !isParamSelect || paramUIType === "select") + (isForm || + !isParamSelect || + paramUIType === "select" || + paramUIType === "cascading") } /> diff --git a/app/src/components/widget-editor/__tests__/parameter-config-section-ui.test.tsx b/app/src/components/widget-editor/__tests__/parameter-config-section-ui.test.tsx index 544c1adf..12d5ffd7 100644 --- a/app/src/components/widget-editor/__tests__/parameter-config-section-ui.test.tsx +++ b/app/src/components/widget-editor/__tests__/parameter-config-section-ui.test.tsx @@ -90,7 +90,13 @@ vi.mock("@neoboard/components", () => ({ vi.mock("lucide-react", () => { const Icon = () => ; - return { Calendar: Icon, Type: Icon, ListFilter: Icon }; + return { + Calendar: Icon, + Type: Icon, + ListFilter: Icon, + SlidersHorizontal: Icon, + GitBranch: Icon, + }; }); const mockSetParamUIType = vi.fn(); @@ -338,4 +344,83 @@ describe("ParameterConfigSection", () => { expect(screen.getByText("$param_period_from")).toBeInTheDocument(); expect(screen.getByText("$param_period_to")).toBeInTheDocument(); }); + + // ── number-range editor (regression: #861) ──────────────────────── + it("shows range-bounds inputs for number-range type", () => { + mockStoreState.paramUIType = "number-range"; + render( + , + ); + expect(screen.getByLabelText("Range minimum")).toBeInTheDocument(); + expect(screen.getByLabelText("Range maximum")).toBeInTheDocument(); + expect(screen.getByLabelText("Range step")).toBeInTheDocument(); + }); + + it("shows number-range sub-parameters in reference hint", () => { + mockStoreState.paramUIType = "number-range"; + mockStoreState.paramWidgetName = "year"; + render( + , + ); + expect(screen.getByText("$param_year_min")).toBeInTheDocument(); + expect(screen.getByText("$param_year_max")).toBeInTheDocument(); + }); + + it("writes rangeMax into chartOptions when user changes max input", () => { + mockStoreState.paramUIType = "number-range"; + mockStoreState.chartOptions = { rangeMin: 0, rangeMax: 100, rangeStep: 1 }; + render( + , + ); + fireEvent.change(screen.getByLabelText("Range maximum"), { + target: { value: "250" }, + }); + // Either a direct object or a functional updater is acceptable — + // we only need to confirm chartOptions was updated for the max field. + expect(mockSetChartOptions).toHaveBeenCalled(); + }); + + // ── cascading editor (regression: #861) ──────────────────────────── + it("shows parent-parameter input for cascading type", () => { + mockStoreState.paramUIType = "cascading"; + render( + , + ); + expect(screen.getByText("Parent Parameter Name")).toBeInTheDocument(); + }); + + it("still shows seed query input for cascading type", () => { + mockStoreState.paramUIType = "cascading"; + render( + , + ); + // The same seed-query block used by `select` should also render here. + expect(screen.getByText("Seed Query")).toBeInTheDocument(); + }); + + it("hides seed query input for number-range type", () => { + mockStoreState.paramUIType = "number-range"; + render( + , + ); + expect(screen.queryByText("Seed Query")).not.toBeInTheDocument(); + }); }); diff --git a/app/src/components/widget-editor/__tests__/parameter-config-section.test.tsx b/app/src/components/widget-editor/__tests__/parameter-config-section.test.tsx index 089a1fb3..ede41bf5 100644 --- a/app/src/components/widget-editor/__tests__/parameter-config-section.test.tsx +++ b/app/src/components/widget-editor/__tests__/parameter-config-section.test.tsx @@ -3,7 +3,13 @@ import { describe, it, expect, vi } from "vitest"; vi.mock("@neoboard/components", () => ({})); vi.mock("lucide-react", () => { const Icon = () => null; - return { Calendar: Icon, Type: Icon, ListFilter: Icon }; + return { + Calendar: Icon, + Type: Icon, + ListFilter: Icon, + SlidersHorizontal: Icon, + GitBranch: Icon, + }; }); vi.mock("@/stores/widget-editor-store", () => ({ useWidgetEditorStore: () => ({}), @@ -43,6 +49,18 @@ describe("resolveInternalParamType", () => { ); }); + it("maps number-range to number-range (regression: #861)", () => { + expect(resolveInternalParamType("number-range", "single", false)).toBe( + "number-range", + ); + }); + + it("maps cascading to cascading-select (regression: #861)", () => { + expect(resolveInternalParamType("cascading", "single", false)).toBe( + "cascading-select", + ); + }); + it("ignores dateSub for non-date types", () => { expect(resolveInternalParamType("freetext", "range", false)).toBe("text"); }); @@ -109,6 +127,22 @@ describe("reverseParamTypeMapping", () => { }); }); + it("maps number-range back (regression: #861)", () => { + expect(reverseParamTypeMapping("number-range")).toEqual({ + uiType: "number-range", + dateSub: "single", + multi: false, + }); + }); + + it("maps cascading-select back (regression: #861)", () => { + expect(reverseParamTypeMapping("cascading-select")).toEqual({ + uiType: "cascading", + dateSub: "single", + multi: false, + }); + }); + it("roundtrips with resolveInternalParamType", () => { const cases = [ { ui: "date" as const, sub: "single" as const, multi: false }, @@ -117,6 +151,8 @@ describe("reverseParamTypeMapping", () => { { ui: "freetext" as const, sub: "single" as const, multi: false }, { ui: "select" as const, sub: "single" as const, multi: false }, { ui: "select" as const, sub: "single" as const, multi: true }, + { ui: "number-range" as const, sub: "single" as const, multi: false }, + { ui: "cascading" as const, sub: "single" as const, multi: false }, ]; for (const { ui, sub, multi } of cases) { const internal = resolveInternalParamType(ui, sub, multi); diff --git a/app/src/components/widget-editor/__tests__/parameter-preview.test.tsx b/app/src/components/widget-editor/__tests__/parameter-preview.test.tsx new file mode 100644 index 00000000..493e5830 --- /dev/null +++ b/app/src/components/widget-editor/__tests__/parameter-preview.test.tsx @@ -0,0 +1,340 @@ +import React from "react"; +import { describe, it, expect, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; + +// Mock @neoboard/components so we render lightweight stand-ins. Each preview +// branch is responsible for rendering its specific widget — we assert the +// right one fires by tagging the mock with a data-testid that carries the +// props we want to inspect. +vi.mock("@neoboard/components", () => ({ + Label: ({ children }: React.PropsWithChildren) => , + TextInputParameter: ({ + parameterName, + placeholder, + }: { + parameterName: string; + placeholder?: string; + }) => ( +
+ ), + DatePickerParameter: ({ parameterName }: { parameterName: string }) => ( +
+ ), + DateRangeParameter: ({ parameterName }: { parameterName: string }) => ( +
+ ), + DateRelativePicker: ({ parameterName }: { parameterName: string }) => ( +
+ ), + ParamSelector: ({ + parameterName, + loading, + options, + placeholder, + }: { + parameterName: string; + loading?: boolean; + options: { value: string; label: string }[]; + placeholder?: string; + }) => ( +
+ ), + ParamMultiSelector: ({ + parameterName, + options, + }: { + parameterName: string; + options: { value: string; label: string }[]; + }) => ( +
+ ), + NumberRangeSlider: ({ + parameterName, + min, + max, + step, + }: { + parameterName: string; + min: number; + max: number; + step: number; + }) => ( +
+ ), + CascadingSelector: ({ + parameterName, + parentParameterName, + placeholder, + }: { + parameterName: string; + parentParameterName?: string; + placeholder?: string; + }) => ( +
+ ), +})); + +import { ParameterPreview } from "../parameter-preview"; + +const baseProps = { + paramUIType: "freetext" as const, + dateSub: "single" as const, + multiSelect: false, + paramWidgetName: "city", + chartOptions: {}, + seedPreviewOptions: null, + seedQueryPending: false, +}; + +describe("ParameterPreview", () => { + it("shows the param name label with $param_ prefix", () => { + render(); + expect(screen.getByText("$param_city")).toBeInTheDocument(); + }); + + it("falls back to a generic label when name is empty", () => { + render(); + expect(screen.getByText("Parameter preview")).toBeInTheDocument(); + }); + + it("renders TextInputParameter for freetext type", () => { + render( + , + ); + const el = screen.getByTestId("text"); + expect(el).toHaveAttribute("data-placeholder", "type here"); + expect(el).toHaveAttribute("data-name", "city"); + }); + + it("renders single date picker when dateSub=single", () => { + render( + , + ); + expect(screen.getByTestId("date-single")).toBeInTheDocument(); + }); + + it("renders date range when dateSub=range", () => { + render( + , + ); + expect(screen.getByTestId("date-range")).toBeInTheDocument(); + }); + + it("renders relative date when dateSub=relative", () => { + render( + , + ); + expect(screen.getByTestId("date-relative")).toBeInTheDocument(); + }); + + it("renders single select when not multiSelect", () => { + render(); + const el = screen.getByTestId("select-single"); + expect(el).toHaveAttribute("data-option-count", "3"); + expect(el).toHaveAttribute("data-loading", "false"); + }); + + it("renders multi-select when multiSelect=true", () => { + render( + , + ); + expect(screen.getByTestId("select-multi")).toBeInTheDocument(); + }); + + it("uses seedPreviewOptions when provided, falling back to defaults otherwise", () => { + const { rerender } = render( + , + ); + expect(screen.getByTestId("select-single")).toHaveAttribute( + "data-option-count", + "1", + ); + rerender( + , + ); + expect(screen.getByTestId("select-single")).toHaveAttribute( + "data-option-count", + "3", + ); + }); + + it("propagates seedQueryPending as loading state", () => { + render( + , + ); + expect(screen.getByTestId("select-single")).toHaveAttribute( + "data-loading", + "true", + ); + }); + + it("renders seedQueryError text when present", () => { + render( + , + ); + expect(screen.getByText("bad SQL")).toBeInTheDocument(); + }); + + describe("number-range branch", () => { + it("renders with chartOptions min/max/step", () => { + render( + , + ); + const el = screen.getByTestId("number-range"); + expect(el).toHaveAttribute("data-min", "5"); + expect(el).toHaveAttribute("data-max", "50"); + expect(el).toHaveAttribute("data-step", "2"); + }); + + it("falls back to defaults (0..100, step 1) when chartOptions are missing", () => { + render(); + const el = screen.getByTestId("number-range"); + expect(el).toHaveAttribute("data-min", "0"); + expect(el).toHaveAttribute("data-max", "100"); + expect(el).toHaveAttribute("data-step", "1"); + }); + + it("guards against max <= min by clamping to min + 1", () => { + // Regression: the slider crashes when min === max — the preview must + // not propagate that into the rendered widget. + render( + , + ); + const el = screen.getByTestId("number-range"); + expect(el).toHaveAttribute("data-min", "10"); + expect(el).toHaveAttribute("data-max", "11"); + }); + + it("guards against max < min", () => { + render( + , + ); + expect(screen.getByTestId("number-range")).toHaveAttribute( + "data-max", + "11", + ); + }); + + it("ignores invalid step (0 or non-numeric)", () => { + render( + , + ); + expect(screen.getByTestId("number-range")).toHaveAttribute( + "data-step", + "1", + ); + }); + + it("ignores non-numeric min/max", () => { + render( + , + ); + const el = screen.getByTestId("number-range"); + expect(el).toHaveAttribute("data-min", "0"); + expect(el).toHaveAttribute("data-max", "100"); + }); + }); + + describe("cascading branch", () => { + it("renders the cascading selector with parent param name", () => { + render( + , + ); + const el = screen.getByTestId("cascading"); + expect(el).toHaveAttribute("data-parent", "region"); + }); + + it("renders the cascading selector without parent when not set", () => { + render(); + const el = screen.getByTestId("cascading"); + expect(el).toHaveAttribute("data-parent", ""); + }); + + it("propagates placeholder from chartOptions", () => { + render( + , + ); + expect(screen.getByTestId("cascading")).toHaveAttribute( + "data-placeholder", + "Pick a city", + ); + }); + }); +}); diff --git a/app/src/components/widget-editor/modal-footer.tsx b/app/src/components/widget-editor/modal-footer.tsx index 048f4809..8124d604 100644 --- a/app/src/components/widget-editor/modal-footer.tsx +++ b/app/src/components/widget-editor/modal-footer.tsx @@ -60,7 +60,7 @@ export function ModalFooter({ disabled={ isParamSelect ? !paramWidgetName.trim() || - (paramUIType === "select" && + ((paramUIType === "select" || paramUIType === "cascading") && (!connectionId || !String(chartOptions.seedQuery ?? "").trim())) : isContentOnly diff --git a/app/src/components/widget-editor/parameter-config-section.tsx b/app/src/components/widget-editor/parameter-config-section.tsx index f0331cdb..ea5c5665 100644 --- a/app/src/components/widget-editor/parameter-config-section.tsx +++ b/app/src/components/widget-editor/parameter-config-section.tsx @@ -2,7 +2,13 @@ import React, { useState, useEffect, useRef } from "react"; import { useWidgetEditorStore } from "@/stores/widget-editor-store"; -import { Calendar, Type, ListFilter } from "lucide-react"; +import { + Calendar, + Type, + ListFilter, + SlidersHorizontal, + GitBranch, +} from "lucide-react"; import type { LucideIcon } from "lucide-react"; import { Button, @@ -18,7 +24,19 @@ import { } from "@neoboard/components"; // ── Parameter type mapping helpers ────────────────────────────────── -export type ParamUIType = "date" | "freetext" | "select"; +// +// `ParamUIType` is the editor's UX-facing taxonomy: each value corresponds +// to a top-level dropdown choice. It's intentionally narrower than the +// runtime `parameterType` (8 values) — `"date"` collapses 3 sub-modes +// into one selector + a sub-radio, and `"select"` collapses single vs +// multi via a checkbox. The remaining runtime types (`number-range`, +// `cascading-select`) get their own top-level entries. +export type ParamUIType = + | "date" + | "freetext" + | "select" + | "number-range" + | "cascading"; export type DateSubType = "single" | "range" | "relative"; export function resolveInternalParamType( @@ -34,6 +52,8 @@ export function resolveInternalParamType( : "date"; } if (ui === "freetext") return "text"; + if (ui === "number-range") return "number-range"; + if (ui === "cascading") return "cascading-select"; return multi ? "multi-select" : "select"; } @@ -53,6 +73,10 @@ export function reverseParamTypeMapping(t: string): { return { uiType: "freetext", dateSub: "single", multi: false }; case "multi-select": return { uiType: "select", dateSub: "single", multi: true }; + case "number-range": + return { uiType: "number-range", dateSub: "single", multi: false }; + case "cascading-select": + return { uiType: "cascading", dateSub: "single", multi: false }; default: return { uiType: "select", dateSub: "single", multi: false }; } @@ -63,6 +87,8 @@ const paramTypeMeta: Record = date: { label: "Date Picker", Icon: Calendar }, freetext: { label: "Freetext", Icon: Type }, select: { label: "Select", Icon: ListFilter }, + "number-range": { label: "Number Range", Icon: SlidersHorizontal }, + cascading: { label: "Cascading Select", Icon: GitBranch }, }; const paramTypes = Object.keys(paramTypeMeta) as ParamUIType[]; @@ -81,15 +107,19 @@ function SeedQueryInput({ placeholder: string; }) { const [draft, setDraft] = useState(value); + // Track the prop value so we can detect external updates (e.g. a parent + // resetting it) and resync the draft *during render*, per + // https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes + const [prevValue, setPrevValue] = useState(value); + if (value !== prevValue) { + setPrevValue(value); + setDraft(value); + } const onChangeRef = useRef(onChange); useEffect(() => { onChangeRef.current = onChange; }, [onChange]); - useEffect(() => { - setDraft(value); - }, [value]); - useEffect(() => { if (draft === value) return; const timer = setTimeout(() => { @@ -202,8 +232,89 @@ export function ParameterConfigSection({
)} - {/* Seed Query (only for select type) */} - {paramUIType === "select" && ( + {/* Number-range bounds (only for number-range) */} + {paramUIType === "number-range" && ( +
+ +
+ + onChartOptionsChange((prev) => ({ + ...prev, + rangeMin: Number(e.target.value), + })) + } + className="w-24" + /> + to + + onChartOptionsChange((prev) => ({ + ...prev, + rangeMax: Number(e.target.value), + })) + } + className="w-24" + /> + step + + onChartOptionsChange((prev) => ({ + ...prev, + rangeStep: Number(e.target.value), + })) + } + className="w-20" + /> +
+

+ Use step ≥ 1 for + integers, or a fractional value (e.g. 0.1) for floats. +

+
+ )} + + {/* Cascading parent (only for cascading) */} + {paramUIType === "cascading" && ( +
+ + + onChartOptionsChange((prev) => ({ + ...prev, + parentParameterName: e.target.value, + })) + } + placeholder="e.g. country" + /> +

+ The seed query below can reference the parent via{" "} + + $param_ + {(chartOptions.parentParameterName as string) || "parent"} + + . The cascade re-runs whenever the parent value changes. +

+
+ )} + + {/* Seed Query (for select and cascading) */} + {(paramUIType === "select" || paramUIType === "cascading") && (
)} diff --git a/app/src/components/widget-editor/parameter-preview.tsx b/app/src/components/widget-editor/parameter-preview.tsx index 9578d365..53ba2387 100644 --- a/app/src/components/widget-editor/parameter-preview.tsx +++ b/app/src/components/widget-editor/parameter-preview.tsx @@ -8,6 +8,8 @@ import { DateRelativePicker, ParamSelector, ParamMultiSelector, + NumberRangeSlider, + CascadingSelector, } from "@neoboard/components"; import type { ParamUIType, DateSubType } from "./parameter-config-section"; @@ -102,6 +104,44 @@ export function ParameterPreview({ placeholder={(chartOptions.placeholder as string) || "Select..."} /> )} + {paramUIType === "number-range" && + (() => { + const rawMin = chartOptions.rangeMin; + const rawMax = chartOptions.rangeMax; + const rawStep = chartOptions.rangeStep; + const min = typeof rawMin === "number" ? rawMin : 0; + // Always keep max > min so the slider renders even when the user + // hasn't typed bounds yet. + const maxCandidate = typeof rawMax === "number" ? rawMax : 100; + const max = maxCandidate > min ? maxCandidate : min + 1; + const step = + typeof rawStep === "number" && rawStep > 0 ? rawStep : 1; + return ( + {}} + onClear={() => {}} + /> + ); + })()} + {paramUIType === "cascading" && ( + {}} + options={seedPreviewOptions ?? DEFAULT_PREVIEW_OPTIONS} + parentParameterName={ + (chartOptions.parentParameterName as string) || undefined + } + parentValue={undefined} + loading={seedQueryPending} + placeholder={(chartOptions.placeholder as string) || undefined} + /> + )}
); diff --git a/app/src/components/widget-editor/use-widget-save.ts b/app/src/components/widget-editor/use-widget-save.ts index 6f7b8e71..d865c525 100644 --- a/app/src/components/widget-editor/use-widget-save.ts +++ b/app/src/components/widget-editor/use-widget-save.ts @@ -60,8 +60,9 @@ export function useBuildWidgetForSave( multiSelect, ), parameterName: paramWidgetName, + // Seed query is only meaningful for the option-backed types. seedQuery: - paramUIType === "select" + paramUIType === "select" || paramUIType === "cascading" ? (chartOptions.seedQuery ?? "") : undefined, } @@ -79,7 +80,12 @@ export function useBuildWidgetForSave( id: existingWidget?.id ?? crypto.randomUUID(), chartType, connectionId: - (isParamSelect && paramUIType !== "select") || isContentOnly + // Option-backed parameter types (select, cascading) need a connection + // to run the seed query. Date/freetext/number-range have no DB query. + (isParamSelect && + paramUIType !== "select" && + paramUIType !== "cascading") || + isContentOnly ? "" : connectionId, query: isParamSelect || isContentOnly ? "" : query, diff --git a/app/src/stores/__tests__/widget-editor-store.test.ts b/app/src/stores/__tests__/widget-editor-store.test.ts index c835d657..1a82e319 100644 --- a/app/src/stores/__tests__/widget-editor-store.test.ts +++ b/app/src/stores/__tests__/widget-editor-store.test.ts @@ -399,6 +399,44 @@ describe("widget-editor-store", () => { expect(getState().paramUIType).toBe("date"); expect(getState().dateSub).toBe("relative"); }); + + it("loads parameter-select with number-range type", () => { + getState().loadFromWidget({ + id: "w1", + chartType: "parameter-select", + connectionId: "c1", + query: "q", + settings: { + chartOptions: { + parameterType: "number-range", + parameterName: "price", + }, + }, + }); + + expect(getState().paramUIType).toBe("number-range"); + expect(getState().multiSelect).toBe(false); + expect(getState().paramWidgetName).toBe("price"); + }); + + it("loads parameter-select with cascading-select type", () => { + getState().loadFromWidget({ + id: "w1", + chartType: "parameter-select", + connectionId: "c1", + query: "q", + settings: { + chartOptions: { + parameterType: "cascading-select", + parameterName: "city", + }, + }, + }); + + expect(getState().paramUIType).toBe("cascading"); + expect(getState().multiSelect).toBe(false); + expect(getState().paramWidgetName).toBe("city"); + }); }); describe("loadFromWidget — form widget fields", () => { diff --git a/app/src/stores/widget-editor-store.ts b/app/src/stores/widget-editor-store.ts index d5619443..8d6f9c9a 100644 --- a/app/src/stores/widget-editor-store.ts +++ b/app/src/stores/widget-editor-store.ts @@ -19,7 +19,14 @@ import type { Transform } from "@/lib/query/data-transforms"; // ParamUIType/DateSubType are string unions — define locally to avoid importing // the React component file (which pulls in @neoboard/components UI barrel). -export type ParamUIType = "date" | "freetext" | "select"; +// Keep in sync with parameter-config-section.tsx — covered by +// parameter-config-section.test.ts which round-trips every internal type. +export type ParamUIType = + | "date" + | "freetext" + | "select" + | "number-range" + | "cascading"; export type DateSubType = "single" | "range" | "relative"; /** Reverse-map an internal parameterType to UI state. Duplicated from parameter-config-section to avoid UI import. */ @@ -39,6 +46,10 @@ function reverseParamTypeMapping(internalType: string): { return { uiType: "freetext", dateSub: "single", multi: false }; case "multi-select": return { uiType: "select", dateSub: "single", multi: true }; + case "number-range": + return { uiType: "number-range", dateSub: "single", multi: false }; + case "cascading-select": + return { uiType: "cascading", dateSub: "single", multi: false }; default: return { uiType: "select", dateSub: "single", multi: false }; }