diff --git a/app/src/components/widget-editor-modal.tsx b/app/src/components/widget-editor-modal.tsx index 8abe2f13..06303f1b 100644 --- a/app/src/components/widget-editor-modal.tsx +++ b/app/src/components/widget-editor-modal.tsx @@ -9,6 +9,7 @@ import React, { useRef, } from "react"; import { useQueryExecution } from "@/hooks/use-query-execution"; +import { allReferencedParamsReady } from "@/hooks/use-widget-query"; import type { DashboardWidget, DashboardLayoutV2, @@ -284,6 +285,12 @@ export function WidgetEditorModal({ const previewQuery = useQueryExecution(); const allParamValues = useParameterValues(); + // Query references $param_x tokens that aren't all bound — the preview shows + // a waiting state instead of running the literal token and erroring (#1055). + const previewWaitingForParams = !allReferencedParamsReady( + query, + allParamValues, + ); // Derive the selected connection object so we can read its type const selectedConnection = useMemo( @@ -890,6 +897,7 @@ export function WidgetEditorModal({ }} initialPreviewData={initialPreviewData} onRunPreview={handlePreview} + waitingForParams={previewWaitingForParams} /> diff --git a/app/src/components/widget-editor/__tests__/use-auto-preview.test.tsx b/app/src/components/widget-editor/__tests__/use-auto-preview.test.tsx index af5b1245..980ff975 100644 --- a/app/src/components/widget-editor/__tests__/use-auto-preview.test.tsx +++ b/app/src/components/widget-editor/__tests__/use-auto-preview.test.tsx @@ -20,6 +20,26 @@ vi.mock("@/hooks/use-widget-query", () => ({ return result; }, ), + // Mirrors the real helper: a query is ready unless it references a + // $param_ that is missing/empty in allParams (#1055). + allReferencedParamsReady: vi.fn( + (q: string, allParams: Record) => { + const regex = /\$param_(\w+)/g; + let match; + while ((match = regex.exec(q)) !== null) { + const v = allParams[match[1]]; + if ( + v === undefined || + v === null || + v === "" || + (Array.isArray(v) && v.length === 0) + ) { + return false; + } + } + return true; + }, + ), })); vi.mock("@/lib/query/wrap-with-preview-limit", () => ({ diff --git a/app/src/components/widget-editor/__tests__/widget-preview-panel.test.tsx b/app/src/components/widget-editor/__tests__/widget-preview-panel.test.tsx index fece0501..9552d405 100644 --- a/app/src/components/widget-editor/__tests__/widget-preview-panel.test.tsx +++ b/app/src/components/widget-editor/__tests__/widget-preview-panel.test.tsx @@ -117,6 +117,22 @@ describe("WidgetPreviewPanel", () => { expect(screen.queryByText("Run")).not.toBeInTheDocument(); }); + it("shows a waiting state instead of running an unbound-param query (#1055)", () => { + render( + , + ); + expect(screen.getByTestId("preview-waiting-params")).toHaveTextContent( + /Waiting for parameters/i, + ); + // The chart preview (card container) must not render while waiting. + expect(screen.queryByTestId("card-container")).not.toBeInTheDocument(); + }); + it("renders MarkdownWidget when isMarkdown", () => { render( s.setParamWidgetName, ); + // The consumed token strips a leading param_ so it isn't doubled (#1055). + const displayParamName = normalizeParamName(paramWidgetName); const chartOptions = useWidgetEditorStore((s) => s.chartOptions); const onChartOptionsChange = useWidgetEditorStore((s) => s.setChartOptions); const connectionId = useWidgetEditorStore((s) => s.connectionId); @@ -387,7 +390,7 @@ export function ParameterConfigSection({

Other widgets can use this parameter as:{" "} - $param_{paramWidgetName} + $param_{displayParamName}

{paramUIType === "date" && @@ -395,11 +398,11 @@ export function ParameterConfigSection({

Date range sub-parameters:{" "} - $param_{paramWidgetName}_from + $param_{displayParamName}_from ,{" "} - $param_{paramWidgetName}_to + $param_{displayParamName}_to

)} @@ -407,11 +410,11 @@ export function ParameterConfigSection({

Number range sub-parameters:{" "} - $param_{paramWidgetName}_min + $param_{displayParamName}_min ,{" "} - $param_{paramWidgetName}_max + $param_{displayParamName}_max

)} diff --git a/app/src/components/widget-editor/parameter-preview.tsx b/app/src/components/widget-editor/parameter-preview.tsx index 53ba2387..fb0c19d4 100644 --- a/app/src/components/widget-editor/parameter-preview.tsx +++ b/app/src/components/widget-editor/parameter-preview.tsx @@ -12,6 +12,7 @@ import { CascadingSelector, } from "@neoboard/components"; import type { ParamUIType, DateSubType } from "./parameter-config-section"; +import { normalizeParamName } from "@/lib/parameter/normalize-param-name"; const DEFAULT_PREVIEW_OPTIONS = [ { value: "option-1", label: "Option 1" }, @@ -47,7 +48,9 @@ export function ParameterPreview({ >
{seedQueryError && (

{seedQueryError}

diff --git a/app/src/components/widget-editor/use-auto-preview.ts b/app/src/components/widget-editor/use-auto-preview.ts index 11787c79..d5c21600 100644 --- a/app/src/components/widget-editor/use-auto-preview.ts +++ b/app/src/components/widget-editor/use-auto-preview.ts @@ -8,7 +8,10 @@ import { useState, } from "react"; import type { ConnectionListItem } from "@/hooks/use-connections"; -import { extractReferencedParams } from "@/hooks/use-widget-query"; +import { + extractReferencedParams, + allReferencedParamsReady, +} from "@/hooks/use-widget-query"; import { wrapWithPreviewLimit } from "@/lib/query/wrap-with-preview-limit"; import type { DashboardWidget } from "@/lib/db/schema"; @@ -78,6 +81,11 @@ export function useAutoPreview({ const cId = connectionIdRef.current; const q = queryRef.current; if (cId && q.trim()) { + // Don't run a query that still has unbound $param_x tokens — the literal + // token would surface a raw `syntax error at or near "$"` in the editor + // preview. Mirror the dashboard's "Waiting for parameters…" state by + // skipping the run (#1055). + if (!allReferencedParamsReady(q, allParamValuesRef.current)) return; const referenced = extractReferencedParams(q, allParamValuesRef.current); const params = Object.keys(referenced).length > 0 ? referenced : undefined; diff --git a/app/src/components/widget-editor/use-widget-save.ts b/app/src/components/widget-editor/use-widget-save.ts index d865c525..0715797e 100644 --- a/app/src/components/widget-editor/use-widget-save.ts +++ b/app/src/components/widget-editor/use-widget-save.ts @@ -4,6 +4,7 @@ import { useCallback } from "react"; import { useWidgetEditorStore } from "@/stores/widget-editor-store"; import type { DashboardWidget, DashboardLayoutV2 } from "@/lib/db/schema"; import { resolveInternalParamType } from "./parameter-config-section"; +import { normalizeParamName } from "@/lib/parameter/normalize-param-name"; /** * Builds a DashboardWidget object from the current widget editor store state. @@ -59,7 +60,8 @@ export function useBuildWidgetForSave( dateSub, multiSelect, ), - parameterName: paramWidgetName, + // Strip a leading param_ so the consumed token isn't doubled (#1055). + parameterName: normalizeParamName(paramWidgetName), // Seed query is only meaningful for the option-backed types. seedQuery: paramUIType === "select" || paramUIType === "cascading" diff --git a/app/src/components/widget-editor/widget-preview-panel.tsx b/app/src/components/widget-editor/widget-preview-panel.tsx index b438657d..fbf19117 100644 --- a/app/src/components/widget-editor/widget-preview-panel.tsx +++ b/app/src/components/widget-editor/widget-preview-panel.tsx @@ -61,6 +61,8 @@ type WidgetPreviewPanelProps = Readonly<{ }; initialPreviewData: PreviewData | undefined; onRunPreview: () => void; + /** Query references $param_x tokens that aren't all bound yet (#1055). */ + waitingForParams?: boolean; }>; function renderMarkdown(chartOptions: Record) { @@ -253,6 +255,7 @@ export function WidgetPreviewPanel({ previewQuery, initialPreviewData, onRunPreview, + waitingForParams, }: WidgetPreviewPanelProps) { function renderPreviewContent() { if (isMarkdown) return renderMarkdown(chartOptions); @@ -270,6 +273,20 @@ export function WidgetPreviewPanel({ }); } if (isForm) return renderForm(formFields, chartOptions); + if (waitingForParams) { + // Mirror the dashboard's waiting state instead of running the literal + // $param_x token and surfacing a raw DB syntax error (#1055). + return ( +
+

+ Waiting for parameters… +

+
+ ); + } return renderChart({ chartType, connectionId, @@ -311,6 +328,7 @@ export function WidgetPreviewPanel({ {!isParamSelect && !isForm && !isContentOnly && + !waitingForParams && previewQuery.isError && ( diff --git a/app/src/lib/parameter/__tests__/normalize-param-name.test.ts b/app/src/lib/parameter/__tests__/normalize-param-name.test.ts new file mode 100644 index 00000000..550f0c1a --- /dev/null +++ b/app/src/lib/parameter/__tests__/normalize-param-name.test.ts @@ -0,0 +1,19 @@ +import { describe, it, expect } from "vitest"; +import { normalizeParamName } from "../normalize-param-name"; + +describe("normalizeParamName (#1055)", () => { + it("strips a leading param_ prefix so the token isn't doubled", () => { + expect(normalizeParamName("param_status")).toBe("status"); + expect(normalizeParamName("PARAM_status")).toBe("status"); + }); + + it("leaves a clean name untouched", () => { + expect(normalizeParamName("status")).toBe("status"); + expect(normalizeParamName("country")).toBe("country"); + }); + + it("only strips one leading prefix (not internal occurrences)", () => { + expect(normalizeParamName("param_param_status")).toBe("param_status"); + expect(normalizeParamName("status_param")).toBe("status_param"); + }); +}); diff --git a/app/src/lib/parameter/normalize-param-name.ts b/app/src/lib/parameter/normalize-param-name.ts new file mode 100644 index 00000000..56fcb716 --- /dev/null +++ b/app/src/lib/parameter/normalize-param-name.ts @@ -0,0 +1,11 @@ +/** + * Normalize a user-supplied parameter name (#1055). + * + * Parameters are consumed as `$param_`, so a user who names a parameter + * `param_status` would otherwise produce a doubled `$param_param_status` token + * and a `PARAM_STATUS` label. Strip a single leading `param_` (case-insensitive) + * so the consumed token and label are clean. + */ +export function normalizeParamName(name: string): string { + return name.replace(/^param_/i, ""); +} diff --git a/component/src/components/composed/__tests__/data-grid-pagination.test.tsx b/component/src/components/composed/__tests__/data-grid-pagination.test.tsx index db054139..28c15777 100644 --- a/component/src/components/composed/__tests__/data-grid-pagination.test.tsx +++ b/component/src/components/composed/__tests__/data-grid-pagination.test.tsx @@ -27,11 +27,24 @@ describe("DataGridPagination", () => { data={data} pageSize={10} pagination={(table) => } - /> + />, ); expect(screen.getByText("Rows per page")).toBeInTheDocument(); }); + it("shows the current page size in the selector trigger (#1055)", () => { + render( + } + />, + ); + // The trigger (combobox) displays the active page size, not just a chevron. + expect(screen.getByRole("combobox")).toHaveTextContent("10"); + }); + it("renders page info", () => { render( { data={data} pageSize={10} pagination={(table) => } - /> + />, ); expect(screen.getByText("Page 1 of 3")).toBeInTheDocument(); }); @@ -52,7 +65,7 @@ describe("DataGridPagination", () => { data={data} pageSize={10} pagination={(table) => } - /> + />, ); await user.click(screen.getByRole("button", { name: "Go to next page" })); expect(screen.getByText("Page 2 of 3")).toBeInTheDocument(); @@ -66,10 +79,12 @@ describe("DataGridPagination", () => { data={data} pageSize={10} pagination={(table) => } - /> + />, ); await user.click(screen.getByRole("button", { name: "Go to next page" })); - await user.click(screen.getByRole("button", { name: "Go to previous page" })); + await user.click( + screen.getByRole("button", { name: "Go to previous page" }), + ); expect(screen.getByText("Page 1 of 3")).toBeInTheDocument(); }); @@ -80,8 +95,10 @@ describe("DataGridPagination", () => { data={data} pageSize={10} pagination={(table) => } - /> + />, ); - expect(screen.getByRole("button", { name: "Go to previous page" })).toBeDisabled(); + expect( + screen.getByRole("button", { name: "Go to previous page" }), + ).toBeDisabled(); }); }); diff --git a/component/src/components/composed/__tests__/data-grid-view-options.test.tsx b/component/src/components/composed/__tests__/data-grid-view-options.test.tsx index 80da3230..df5b92b7 100644 --- a/component/src/components/composed/__tests__/data-grid-view-options.test.tsx +++ b/component/src/components/composed/__tests__/data-grid-view-options.test.tsx @@ -1,5 +1,6 @@ import { render, screen } from "@testing-library/react"; import { describe, it, expect } from "vitest"; +import userEvent from "@testing-library/user-event"; import type { ColumnDef } from "@tanstack/react-table"; import { DataGrid } from "../data-grid"; import { DataGridViewOptions } from "../data-grid-view-options"; @@ -7,14 +8,18 @@ import { DataGridViewOptions } from "../data-grid-view-options"; interface TestRow { name: string; email: string; + total_spend: number; } const columns: ColumnDef[] = [ { accessorKey: "name", header: "Name" }, { accessorKey: "email", header: "Email" }, + { accessorKey: "total_spend", header: "Total Spend" }, ]; -const data: TestRow[] = [{ name: "Alice", email: "alice@example.com" }]; +const data: TestRow[] = [ + { name: "Alice", email: "alice@example.com", total_spend: 100 }, +]; describe("DataGridViewOptions", () => { it("renders icon-only button with sr-only text and title", () => { @@ -35,6 +40,23 @@ describe("DataGridViewOptions", () => { expect(button).toHaveAttribute("title", "Hide columns"); }); + it("humanizes snake_case column labels in the hide-columns menu (#1055)", async () => { + const user = userEvent.setup(); + render( + } + />, + ); + await user.click(screen.getByRole("button", { name: /hide columns/i })); + // total_spend → "Total Spend", not "total_spend". + expect( + screen.getByRole("menuitemcheckbox", { name: "Total Spend" }), + ).toBeInTheDocument(); + expect(screen.queryByText("total_spend")).not.toBeInTheDocument(); + }); + it("does not render visible 'View' label text", () => { render( { expect(screen.getByText("Charlie")).toBeInTheDocument(); }); + it("gives non-grouped data rows a hover affordance (#1055)", () => { + render(); + const dataRow = screen.getByText("Alice").closest("tr"); + expect(dataRow).toHaveClass("hover:bg-muted/40"); + }); + it("shows empty state when no data", () => { render(); expect(screen.getByText("No results.")).toBeInTheDocument(); diff --git a/component/src/components/composed/data-grid-pagination.tsx b/component/src/components/composed/data-grid-pagination.tsx index a00c9175..62fc9e1d 100644 --- a/component/src/components/composed/data-grid-pagination.tsx +++ b/component/src/components/composed/data-grid-pagination.tsx @@ -43,7 +43,9 @@ function DataGridPagination({ }} > - + {/* Render the active page size explicitly — the placeholder only + shows when empty, leaving the trigger blank (#1055). */} + {table.getState().pagination.pageSize} {pageSizeOptions.map((pageSize) => ( diff --git a/component/src/components/composed/data-grid-view-options.tsx b/component/src/components/composed/data-grid-view-options.tsx index 33a3a361..a63ce516 100644 --- a/component/src/components/composed/data-grid-view-options.tsx +++ b/component/src/components/composed/data-grid-view-options.tsx @@ -9,6 +9,7 @@ import { DropdownMenuSeparator, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; +import { humanizeHeader } from "@/lib/humanize-header"; interface DataGridViewOptionsProps { table: Table; @@ -42,11 +43,10 @@ function DataGridViewOptions({ .map((column) => ( column.toggleVisibility(!!value)} > - {column.id} + {humanizeHeader(column.id)} ))} diff --git a/component/src/components/composed/data-grid.tsx b/component/src/components/composed/data-grid.tsx index 927c858f..5b72e743 100644 --- a/component/src/components/composed/data-grid.tsx +++ b/component/src/components/composed/data-grid.tsx @@ -373,9 +373,12 @@ function DataGrid({ key={row.id} data-state={row.getIsSelected() && "selected"} style={!isGrouped ? getRowStyle?.(row.original) : undefined} - className={ - isGrouped ? "bg-muted/50 font-medium" : undefined - } + className={cn( + isGrouped + ? "bg-muted/50 font-medium" + : // Subtle hover affordance on data rows (#1055). + "transition-colors hover:bg-muted/40", + )} > {row.getVisibleCells().map((cell) => { const isDataCell = cell.column.id !== "select"; diff --git a/component/src/lib/__tests__/humanize-header.test.ts b/component/src/lib/__tests__/humanize-header.test.ts new file mode 100644 index 00000000..d591e9a7 --- /dev/null +++ b/component/src/lib/__tests__/humanize-header.test.ts @@ -0,0 +1,19 @@ +import { describe, it, expect } from "vitest"; +import { humanizeHeader } from "../humanize-header"; + +describe("humanizeHeader (#1055)", () => { + it("spaces and capitalizes snake_case", () => { + expect(humanizeHeader("Total_spend")).toBe("Total Spend"); + expect(humanizeHeader("total_spend")).toBe("Total Spend"); + }); + + it("capitalizes a single word", () => { + expect(humanizeHeader("customer")).toBe("Customer"); + expect(humanizeHeader("City")).toBe("City"); + }); + + it("collapses repeated separators", () => { + expect(humanizeHeader("first__name")).toBe("First Name"); + expect(humanizeHeader("a b")).toBe("A B"); + }); +}); diff --git a/component/src/lib/humanize-header.ts b/component/src/lib/humanize-header.ts new file mode 100644 index 00000000..85241b58 --- /dev/null +++ b/component/src/lib/humanize-header.ts @@ -0,0 +1,14 @@ +/** + * Humanize a raw column id into a display label (#1055). + * + * Splits snake_case (and spaces) into words and Capitalizes each, so a column + * like `Total_spend` reads "Total Spend", consistent with "Customer"/"City". + * Already-spaced or single words pass through Capitalized. + */ +export function humanizeHeader(id: string): string { + return id + .split(/[_\s]+/) + .filter(Boolean) + .map((w) => w.charAt(0).toUpperCase() + w.slice(1)) + .join(" "); +}