diff --git a/packages/components/package.json b/packages/components/package.json index 2e4ff27ed8..2c42617e50 100644 --- a/packages/components/package.json +++ b/packages/components/package.json @@ -90,6 +90,7 @@ "tailwindcss": "^4.2.1" }, "devDependencies": { + "@objectstack/spec": "^17.0.0-rc.0", "@tailwindcss/postcss": "^4.3.3", "@types/react": "19.2.17", "@types/react-dom": "19.2.3", @@ -100,7 +101,8 @@ "tailwindcss": "^4.3.3", "typescript": "^6.0.3", "vite": "^8.1.5", - "vite-plugin-dts": "^5.0.3" + "vite-plugin-dts": "^5.0.3", + "zod": "^4.4.3" }, "keywords": [ "objectui", diff --git a/packages/components/src/__tests__/data-table-selection-mode.test.tsx b/packages/components/src/__tests__/data-table-selection-mode.test.tsx new file mode 100644 index 0000000000..4e01e8493c --- /dev/null +++ b/packages/components/src/__tests__/data-table-selection-mode.test.tsx @@ -0,0 +1,150 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Selection mode ↔ spec vocabulary parity + behavior (#2941). + * + * `SelectionConfigSchema.type` (`ui/view.zod.ts`) publishes three names: + * `none | single | multiple`. The table used to treat `selectable` as a bare + * truthy, so a view authored with `selection.type: 'single'` rendered the full + * multi-select UX — per-row checkboxes that accumulate AND a select-all + * header. The output looked right and wasn't: `single` was never + * distinguished from `multiple`. + * + * Contract under test: + * - parity: the table's declared mode set equals the spec enum, both ways; + * - `single` offers no select-all and replaces the selection on each pick; + * - `multiple` (and legacy `true`) keeps the accumulate + select-all UX. + */ +import { describe, it, expect, vi, beforeAll } from 'vitest'; +import { fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { SelectionConfigSchema } from '@objectstack/spec/ui'; +import { renderComponent } from './test-utils'; +import { SUPPORTED_SELECTION_MODES } from '../renderers/complex/data-table'; + +beforeAll(async () => { + await import('../renderers'); +}, 30000); + +/** The spec's selection vocabulary, read through the `.default()` wrapper. */ +function specSelectionModes(): string[] { + const typeSchema = (SelectionConfigSchema as unknown as { shape?: Record }) + .shape?.type as { def?: { innerType?: { options?: readonly string[] } } } | undefined; + const options = typeSchema?.def?.innerType?.options; + return Array.isArray(options) ? [...options] : []; +} + +const baseSchema = { + type: 'data-table' as const, + columns: [{ header: 'Name', accessorKey: 'name' }], + data: [ + { id: '1', name: 'Alice' }, + { id: '2', name: 'Bob' }, + { id: '3', name: 'Carol' }, + ], + pagination: false, + searchable: false, +}; + +describe('data-table selection mode covers the spec selection vocabulary', () => { + const specNames = specSelectionModes(); + + it('reads a non-empty enum from the spec', () => { + expect(specNames, 'could not read SelectionConfigSchema.shape.type options from the spec').not.toEqual([]); + }); + + it('implements every selection mode the spec accepts', () => { + const unimplemented = specNames.filter((name) => !SUPPORTED_SELECTION_MODES.has(name)); + expect( + unimplemented, + 'these pass schema validation but render an undistinguished selection UX — implement them in data-table', + ).toEqual([]); + }); + + it('does not accept selection modes the spec rejects', () => { + const extra = [...SUPPORTED_SELECTION_MODES].filter((name) => !specNames.includes(name)); + expect( + extra, + 'these are renderer-local dialect — promote them into @objectstack/spec instead', + ).toEqual([]); + }); +}); + +describe('data-table selection behavior per mode', () => { + it("'single' renders no select-all header and replaces the selection on each pick", () => { + const onSelectionChange = vi.fn(); + const { getAllByRole } = renderComponent({ + ...baseSchema, + selectable: 'single', + onSelectionChange, + } as any); + + // 3 rows → exactly 3 checkboxes; a select-all header would make it 4. + const checkboxes = getAllByRole('checkbox'); + expect(checkboxes).toHaveLength(3); + + fireEvent.click(checkboxes[0]); + expect(onSelectionChange).toHaveBeenLastCalledWith([expect.objectContaining({ name: 'Alice' })]); + + // Picking Bob must REPLACE Alice, never accumulate to two rows. + fireEvent.click(checkboxes[1]); + expect(onSelectionChange).toHaveBeenLastCalledWith([expect.objectContaining({ name: 'Bob' })]); + }); + + it("'single' allows deselecting the picked row", () => { + const onSelectionChange = vi.fn(); + const { getAllByRole } = renderComponent({ + ...baseSchema, + selectable: 'single', + onSelectionChange, + } as any); + + const checkboxes = getAllByRole('checkbox'); + fireEvent.click(checkboxes[0]); + fireEvent.click(checkboxes[0]); + expect(onSelectionChange).toHaveBeenLastCalledWith([]); + }); + + it("'multiple' keeps the select-all header and accumulates picks", () => { + const onSelectionChange = vi.fn(); + const { getAllByRole } = renderComponent({ + ...baseSchema, + selectable: 'multiple', + onSelectionChange, + } as any); + + // 3 rows + the select-all header. + const checkboxes = getAllByRole('checkbox'); + expect(checkboxes).toHaveLength(4); + + // First checkbox is the header select-all; rows follow. + fireEvent.click(checkboxes[1]); + fireEvent.click(checkboxes[2]); + expect(onSelectionChange).toHaveBeenLastCalledWith([ + expect.objectContaining({ name: 'Alice' }), + expect.objectContaining({ name: 'Bob' }), + ]); + }); + + it('legacy `selectable: true` still means multi-select', () => { + const { getAllByRole } = renderComponent({ + ...baseSchema, + selectable: true, + } as any); + expect(getAllByRole('checkbox')).toHaveLength(4); + }); + + it("'none' renders no selection column", () => { + const { queryAllByRole } = renderComponent({ + ...baseSchema, + selectable: 'none', + } as any); + expect(queryAllByRole('checkbox')).toHaveLength(0); + }); +}); diff --git a/packages/components/src/renderers/complex/data-table.tsx b/packages/components/src/renderers/complex/data-table.tsx index 1e466f4b95..09cb9abf2c 100644 --- a/packages/components/src/renderers/complex/data-table.tsx +++ b/packages/components/src/renderers/complex/data-table.tsx @@ -288,6 +288,30 @@ export const DataTableBuiltinRowActionItem: React.FC<{ ); }; +/** + * The selection modes this table implements — the renderer half of the spec's + * `SelectionConfigSchema.type` vocabulary (`ui/view.zod.ts`). Kept as an + * explicit export so a parity test can fail the moment either side moves + * (#2941; template: plugin-grid `summary-spec-parity.test.ts`). + */ +export const SUPPORTED_SELECTION_MODES: ReadonlySet = new Set(['none', 'single', 'multiple']); + +type SelectionMode = 'none' | 'single' | 'multiple'; + +/** + * Resolve the `selectable` prop — `boolean | 'single' | 'multiple'` (plus + * `'none'` arriving verbatim from raw SDUI JSON) — to a selection mode. + * `'single'` is a real mode (replace-on-select, no select-all), not a truthy + * alias for `'multiple'`; collapsing the two rendered per-row checkboxes AND + * a select-all header for `selection.type: 'single'` (#2941). Legacy `true` + * and unrecognized truthy strings keep their historical multi-select meaning. + */ +function resolveSelectionMode(selectable: DataTableSchema['selectable'] | 'none'): SelectionMode { + if (selectable === 'single') return 'single'; + if (selectable === 'none' || !selectable) return 'none'; + return 'multiple'; +} + /** * Enterprise-level data table component with Airtable-like features. * @@ -295,7 +319,8 @@ export const DataTableBuiltinRowActionItem: React.FC<{ * - Multi-column sorting (ascending/descending/none) * - Real-time search across all columns * - Pagination with configurable page sizes - * - Row selection with persistence across pages + * - Row selection with persistence across pages (multi-select), or + * replace-on-select when the view declares `selection.type: 'single'` * - CSV export of filtered/sorted data * - Row action buttons (edit/delete) * @@ -337,7 +362,7 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { onPageChange, onPageSizeChange, searchable = true, - selectable = false, + selectable: selectableProp = false, showSelectionCount = true, selectionResetKey, sortable = true, @@ -359,6 +384,11 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { disableInnerScroll = false, } = schema; + // 'single' caps the selection at one row (replace-on-select) and drops the + // select-all header; every truthy legacy value keeps meaning 'multiple'. + const selectionMode = resolveSelectionMode(selectableProp); + const selectable = selectionMode !== 'none'; + // Ambient design-surface affordance: when a host (Studio) provides it, render // a trailing "+ add field" column header. `null` for every runtime table, so // existing tables render unchanged. @@ -699,7 +729,9 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { }; const handleSelectRow = (rowId: any, checked: boolean) => { - const newSelected = new Set(selectedRowIds); + // Single mode replaces the previous selection instead of accumulating — + // the spec's 'single' must never hold two rows (#2941). + const newSelected = new Set(selectionMode === 'single' ? [] : selectedRowIds); if (checked) { newSelected.add(rowId); } else { @@ -1283,10 +1315,15 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { {selectable && ( 0 && "sticky left-0 z-20")}> - + {/* Select-all is a multi-select affordance; a 'single' view + keeps the column (alignment) but offers no way to select + more than one row (#2941). */} + {selectionMode === 'multiple' && ( + + )} )} {showRowNumbers && ( diff --git a/packages/plugin-dashboard/package.json b/packages/plugin-dashboard/package.json index afc97c8549..d35a8dee6a 100644 --- a/packages/plugin-dashboard/package.json +++ b/packages/plugin-dashboard/package.json @@ -42,6 +42,7 @@ "react-grid-layout": "^2.2.0 || ^1.4.0" }, "devDependencies": { + "@objectstack/spec": "^17.0.0-rc.0", "@types/react-grid-layout": "^2.1.0", "@vitejs/plugin-react": "^6.0.4", "react-grid-layout": "^2.2.3", diff --git a/packages/plugin-dashboard/src/PivotTable.tsx b/packages/plugin-dashboard/src/PivotTable.tsx index 577f7de9df..aac0ca01e3 100644 --- a/packages/plugin-dashboard/src/PivotTable.tsx +++ b/packages/plugin-dashboard/src/PivotTable.tsx @@ -97,6 +97,17 @@ function displayKey(key: string, labels?: Record): string { return labels?.[key] ?? key; } +/** + * The aggregations this pivot computes — the renderer half of the spec's + * `ChartAggregateFunctionSchema` (`ui/chart.zod.ts`), the UI-side subset the + * spec deliberately carved out of the engine's 8-name `AggregationFunction`. + * Engine-level names (`count_distinct`, `array_agg`, `string_agg`) used to + * fall into a `default:` branch that returned a SUM — a plausible wrong total + * with no signal (#2941). They now short-circuit to a visible notice before + * any cell is computed. Exported for the spec-parity test. + */ +export const PIVOT_AGGREGATIONS: ReadonlySet = new Set(['sum', 'count', 'avg', 'min', 'max']); + /** Aggregate an array of numbers with the given function. */ function aggregate(values: number[], fn: PivotAggregation): number { if (values.length === 0) return 0; @@ -112,7 +123,10 @@ function aggregate(values: number[], fn: PivotAggregation): number { case 'max': return Math.max(...values); default: - return values.reduce((a, b) => a + b, 0); + // Unreachable: the component refuses to render out-of-vocabulary + // aggregations. NaN (never a silent sum) is the tripwire if a new call + // site skips that gate. + return Number.NaN; } } @@ -228,6 +242,26 @@ export const PivotTable: React.FC = ({ schema, className, rowLa const fmt = (v: number) => formatValue(v, format); + // Out-of-vocabulary aggregation (e.g. the engine-level `count_distinct` + // arriving through untyped SDUI JSON): refuse loudly instead of quietly + // summing every cell (#2941). Placed after the hooks so their order stays + // stable across renders. + if (!PIVOT_AGGREGATIONS.has(aggregation)) { + return ( +
+ {title && ( +

{title}

+ )} +
+

+ Unsupported aggregation “{String(aggregation)}” — this pivot table renders{' '} + {[...PIVOT_AGGREGATIONS].join(', ')}. +

+
+
+ ); + } + if (data.length === 0) { return (
diff --git a/packages/plugin-dashboard/src/__tests__/pivot-aggregation-spec-parity.test.tsx b/packages/plugin-dashboard/src/__tests__/pivot-aggregation-spec-parity.test.tsx new file mode 100644 index 0000000000..c480ea077e --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/pivot-aggregation-spec-parity.test.tsx @@ -0,0 +1,93 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Pivot aggregation ↔ spec vocabulary parity + behavior (#2941). + * + * `ChartAggregateFunctionSchema` (`ui/chart.zod.ts`) is the UI-side + * aggregation vocabulary — the five names the spec deliberately carved out of + * the engine's 8-name `AggregationFunction` because client renderers + * implement exactly these. The pivot's `default:` branch used to return a + * SUM for anything else, so `aggregation: 'count_distinct'` (an engine name + * that validates on engine surfaces) produced a plausible wrong total with + * no signal anywhere. + * + * Contract under test: + * - parity: the pivot's implemented set equals the spec enum, both ways; + * - an out-of-vocabulary aggregation renders a visible notice, never numbers. + */ +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { ChartAggregateFunctionSchema } from '@objectstack/spec/ui'; +import { PivotTable, PIVOT_AGGREGATIONS } from '../PivotTable'; + +const baseSchema = { + type: 'pivot' as const, + rowField: 'region', + columnField: 'quarter', + valueField: 'amount', + data: [ + { region: 'East', quarter: 'Q1', amount: 10 }, + { region: 'East', quarter: 'Q1', amount: 10 }, + { region: 'West', quarter: 'Q1', amount: 5 }, + ], +}; + +describe('PivotTable covers the spec UI aggregation vocabulary', () => { + const rawOptions = (ChartAggregateFunctionSchema as unknown as { options?: readonly string[] }).options; + const specNames: string[] = Array.isArray(rawOptions) ? [...rawOptions] : []; + + it('reads a non-empty enum from the spec', () => { + expect(specNames, 'could not read ChartAggregateFunctionSchema.options from the spec').not.toEqual([]); + }); + + it('implements every aggregation the spec accepts', () => { + const unimplemented = specNames.filter((name) => !PIVOT_AGGREGATIONS.has(name)); + expect( + unimplemented, + 'these pass schema validation but refuse to render — implement them in PivotTable.aggregate', + ).toEqual([]); + }); + + it('does not accept aggregations the spec rejects', () => { + const extra = [...PIVOT_AGGREGATIONS].filter((name) => !specNames.includes(name)); + expect( + extra, + 'these are renderer-local dialect — promote them into @objectstack/spec instead', + ).toEqual([]); + }); +}); + +describe('PivotTable refuses out-of-vocabulary aggregations', () => { + it("renders a visible notice for 'count_distinct' instead of silently summing", () => { + render(); + + expect(screen.getByTestId('pivot-unsupported-aggregation')).toBeInTheDocument(); + expect(screen.getByRole('alert').textContent).toContain('count_distinct'); + // The wrong-total failure mode: 2×10 rows would sum to "20". + expect(screen.queryByText('20')).not.toBeInTheDocument(); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); + }); + + it('still renders every supported aggregation as a table', () => { + for (const aggregation of PIVOT_AGGREGATIONS) { + const { unmount, container } = render( + , + ); + expect(container.querySelector('table'), `aggregation '${aggregation}' should render a table`).toBeTruthy(); + unmount(); + } + }); + + it('defaults to sum when no aggregation is authored', () => { + render(); + expect(screen.getByText('20')).toBeInTheDocument(); + }); +}); diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index ff81fb8d08..8e3c079ef5 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -1620,6 +1620,10 @@ export const ObjectGrid: React.FC = ({ // Auto-enable multi-select when bulk actions exist selectionMode = 'multiple'; } + // `selection.type: 'single'` caps the selection at one row (the data-table + // enforces replace-on-select, #2941), so the cross-page "select all N + // matching" escalation must never be offered. + const singleSelection = selectionMode === 'single'; // Resolve the rows the bulk action should actually operate on. When // "select all N matching" is active, fan out a paged find against the @@ -2522,9 +2526,9 @@ export const ObjectGrid: React.FC = ({ onActionDef={dispatchBulkActionDef} onClearSelection={() => { setSelectedRows([]); setSelectAllMatching(false); }} pageSize={data.length} - totalMatching={totalMatching} + totalMatching={singleSelection ? undefined : totalMatching} allMatchingSelected={selectAllMatching} - onSelectAllMatching={() => setSelectAllMatching(true)} + onSelectAllMatching={singleSelection ? undefined : () => setSelectAllMatching(true)} />
} @@ -2559,9 +2563,9 @@ export const ObjectGrid: React.FC = ({ onActionDef={dispatchBulkActionDef} onClearSelection={() => { setSelectedRows([]); setSelectAllMatching(false); }} pageSize={data.length} - totalMatching={totalMatching} + totalMatching={singleSelection ? undefined : totalMatching} allMatchingSelected={selectAllMatching} - onSelectAllMatching={() => setSelectAllMatching(true)} + onSelectAllMatching={singleSelection ? undefined : () => setSelectAllMatching(true)} /> {navigation.isOverlay && ( c.toUpperCase()); } +/** + * Resolve the spec `AddRecordConfigSchema.position` (`ui/view.zod.ts`: + * `top | bottom | both`) to the two render slots. A binary ternary used to + * collapse `both` to `top`, so the bottom button never rendered (#2941). + * + * An absent or unrecognized value keeps this renderer's historical `top` + * placement. (The spec's own default is `bottom`; spec-parsed metadata + * arrives with it materialized, but raw stored JSON predating the spec + * default relied on `top` — moving it is a UX change out of scope here.) + * + * Exported for the spec-parity test, which fails the moment the spec's + * position vocabulary and this mapping drift in either direction. + */ +export function resolveAddRecordPlacement(position: unknown): { top: boolean; bottom: boolean } { + switch (position) { + case 'bottom': + return { top: false, bottom: true }; + case 'both': + return { top: true, bottom: true }; + case 'top': + default: + return { top: true, bottom: false }; + } +} + /** * Normalize an array of filter conditions, expanding `in`/`not in` operators * and ensuring consistent AST structure. @@ -414,6 +439,7 @@ export const ListView = React.forwardRef(({ // buttons on every existing view.) const ua = schema.userActions as Record | undefined; const addRecordEnabled = schema.addRecord?.enabled === true && ua?.addRecordForm !== false; + const addRecordPlacement = resolveAddRecordPlacement(schema.addRecord?.position); return { showSearch: ua?.search !== false, showSort: ua?.sort !== false, @@ -424,8 +450,12 @@ export const ListView = React.forwardRef(({ showHideFields: ua?.hideFields === true, showColor: ua?.rowColor === true, compactToolbar: schema.compactToolbar === true, + // Position-independent switch (empty-state CTA) + the two placement + // slots. `position: 'both'` renders both buttons — it used to collapse + // to `top` through a binary ternary (#2941). showAddRecord: addRecordEnabled, - addRecordPosition: (schema.addRecord?.position === 'bottom' ? 'bottom' : 'top') as 'top' | 'bottom', + showAddRecordTop: addRecordEnabled && addRecordPlacement.top, + showAddRecordBottom: addRecordEnabled && addRecordPlacement.bottom, }; }, [schema.userActions, schema.compactToolbar, schema.addRecord]); @@ -2367,7 +2397,7 @@ export const ListView = React.forwardRef(({ )} {/* Add Record (top position) */} - {toolbarFlags.showAddRecord && toolbarFlags.addRecordPosition === 'top' && ( + {toolbarFlags.showAddRecordTop && (