feat: PivotTable component (cross-tabulation / pivot table)#592
feat: PivotTable component (cross-tabulation / pivot table)#592
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ration Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a PivotTable component to the @object-ui/plugin-dashboard package, enabling cross-tabulation visualization where data is aggregated across two dimensions (rows and columns). The component supports multiple aggregation functions (sum, count, avg, min, max), row and column totals, number formatting, and column-specific color styling.
Changes:
- Added
PivotTableSchemaandPivotAggregationtypes to@object-ui/types - Implemented
PivotTablecomponent with memoized pivot computation - Registered component with ComponentRegistry as
'pivot'widget type - Integrated shorthand widget support in DashboardRenderer
- Added 11 unit tests covering aggregations, totals, formatting, and edge cases
- Updated ROADMAP.md to mark feature as completed
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/types/src/index.ts | Exports new PivotAggregation and PivotTableSchema types |
| packages/types/src/data-display.ts | Defines PivotTableSchema interface with rowField, columnField, valueField, aggregation, totals, format, and columnColors properties |
| packages/plugin-dashboard/src/index.tsx | Registers PivotTable component with ComponentRegistry and exports it from package |
| packages/plugin-dashboard/src/PivotTable.tsx | Implements PivotTable component with pivot computation logic, formatting, and rendering |
| packages/plugin-dashboard/src/tests/PivotTable.test.tsx | Adds 11 unit tests covering aggregation modes, totals, formatting, and edge cases |
| packages/plugin-dashboard/src/DashboardRenderer.tsx | Adds 'pivot' widget shorthand mapping with data extraction pattern |
| ROADMAP.md | Marks PivotTable feature as completed under P1.3 Advanced View Features |
| const { rowKeys, colKeys, matrix, rowTotals, colTotals, grandTotal } = useMemo(() => { | ||
| // Collect unique row/column values preserving insertion order | ||
| const rowSet = new Map<string, true>(); | ||
| const colSet = new Map<string, true>(); | ||
| // Bucket raw values: bucket[row][col] = number[] | ||
| const bucket: Record<string, Record<string, number[]>> = {}; | ||
|
|
||
| for (const item of data) { | ||
| const r = String(item[rowField] ?? ''); | ||
| const c = String(item[columnField] ?? ''); | ||
| const v = Number(item[valueField]) || 0; | ||
|
|
||
| rowSet.set(r, true); | ||
| colSet.set(c, true); | ||
|
|
||
| if (!bucket[r]) bucket[r] = {}; | ||
| if (!bucket[r][c]) bucket[r][c] = []; | ||
| bucket[r][c].push(v); | ||
| } | ||
|
|
||
| const rKeys = Array.from(rowSet.keys()); | ||
| const cKeys = Array.from(colSet.keys()); | ||
|
|
||
| // Build aggregated matrix | ||
| const mat: Record<string, Record<string, number>> = {}; | ||
| const rTotals: Record<string, number> = {}; | ||
| const cTotals: Record<string, number> = {}; | ||
|
|
||
| for (const r of rKeys) { | ||
| mat[r] = {}; | ||
| const rowValues: number[] = []; | ||
| for (const c of cKeys) { | ||
| const cellValues = bucket[r]?.[c] ?? []; | ||
| const cellAgg = aggregate(cellValues, aggregation); | ||
| mat[r][c] = cellAgg; | ||
| rowValues.push(...cellValues); | ||
|
|
||
| // Accumulate column bucket values for column totals | ||
| if (!cTotals[c] && cTotals[c] !== 0) { | ||
| // Will compute after | ||
| } | ||
| } | ||
| rTotals[r] = aggregate(rowValues, aggregation); | ||
| } | ||
|
|
||
| // Column totals | ||
| for (const c of cKeys) { | ||
| const colValues: number[] = []; | ||
| for (const r of rKeys) { | ||
| const cellValues = bucket[r]?.[c] ?? []; | ||
| colValues.push(...cellValues); | ||
| } | ||
| cTotals[c] = aggregate(colValues, aggregation); | ||
| } | ||
|
|
||
| // Grand total | ||
| const allValues: number[] = []; | ||
| for (const item of data) { | ||
| allValues.push(Number(item[valueField]) || 0); | ||
| } | ||
| const gt = aggregate(allValues, aggregation); | ||
|
|
||
| return { rowKeys: rKeys, colKeys: cKeys, matrix: mat, rowTotals: rTotals, colTotals: cTotals, grandTotal: gt }; | ||
| }, [data, rowField, columnField, valueField, aggregation]); |
There was a problem hiding this comment.
The pivot computation has O(n*m) complexity where n is the number of data rows and m is the number of unique row/column combinations. While this is acceptable for typical dashboard use cases, consider documenting performance characteristics or adding a warning if the dataset is very large (e.g., > 10,000 rows). The computation is already memoized which helps, but users should be aware of the computational cost for large datasets.
| import { PivotTable } from './PivotTable'; | ||
|
|
||
| export { DashboardRenderer, DashboardGridLayout, MetricWidget, MetricCard }; | ||
| export { DashboardRenderer, DashboardGridLayout, MetricWidget, MetricCard, PivotTable }; |
There was a problem hiding this comment.
According to Guideline #2 (Documentation Driven Development), documentation must be updated for every feature. The plugin-dashboard README and content/docs/plugins/plugin-dashboard.mdx should document the new PivotTable component with schema API details and usage examples. Additionally, a Storybook story should be added (either a new PivotTable.stories.tsx or an example in DashboardRenderer.stories.tsx) demonstrating the pivot table with different configurations.
|
|
||
| // Accumulate column bucket values for column totals | ||
| if (!cTotals[c] && cTotals[c] !== 0) { | ||
| // Will compute after | ||
| } |
There was a problem hiding this comment.
Dead code detected: lines 143-145 check a condition but do nothing. This code appears to be a leftover from development and should be removed as it serves no purpose and makes the code harder to understand.
| // Accumulate column bucket values for column totals | |
| if (!cTotals[c] && cTotals[c] !== 0) { | |
| // Will compute after | |
| } |
| // comma inside the prefix-ish area means grouping, not a literal prefix | ||
| const raw = prefixMatch[1]; | ||
| prefix = raw.replace(',', ''); | ||
| if (raw.includes(',')) useGrouping = true; |
There was a problem hiding this comment.
The formatValue function's prefix extraction logic at line 34 removes commas from the prefix (e.g., "$," becomes "$"), which is correct. However, this logic is fragile because it treats any comma in the prefix area as a grouping indicator. For example, if a user wants a literal prefix containing a comma (unlikely but possible), this would fail. Consider documenting this behavior in a comment or adding a more explicit parsing strategy.
| // comma inside the prefix-ish area means grouping, not a literal prefix | |
| const raw = prefixMatch[1]; | |
| prefix = raw.replace(',', ''); | |
| if (raw.includes(',')) useGrouping = true; | |
| // By default, a comma in the prefix-ish area is treated as a grouping indicator, | |
| // not a literal prefix character. To render a literal comma, escape it as "\,". | |
| const raw = prefixMatch[1]; | |
| let parsedPrefix = ''; | |
| let escaping = false; | |
| for (let i = 0; i < raw.length; i++) { | |
| const ch = raw[i]; | |
| if (escaping) { | |
| // Only comma is special; other escaped chars are passed through as-is | |
| if (ch === ',') { | |
| parsedPrefix += ','; | |
| } else { | |
| parsedPrefix += '\\' + ch; | |
| } | |
| escaping = false; | |
| continue; | |
| } | |
| if (ch === '\\') { | |
| escaping = true; | |
| continue; | |
| } | |
| if (ch === ',') { | |
| useGrouping = true; | |
| // do not add unescaped comma to prefix; it only signals grouping | |
| continue; | |
| } | |
| parsedPrefix += ch; | |
| } | |
| prefix = parsedPrefix; |
| // Alice + Discovery = avg(1000, 500) = 750 | ||
| expect(screen.getByText('750')).toBeInTheDocument(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Missing test coverage for 'min' and 'max' aggregation functions. While tests cover sum, count, and avg aggregations, the min and max cases are not tested. Consider adding test cases to ensure these aggregation modes work correctly, especially with edge cases like single values or negative numbers.
| it('should support min aggregation', () => { | |
| render(<PivotTable schema={makeSchema({ aggregation: 'min' })} />); | |
| // Alice + Discovery = min(1000, 500) = 500 | |
| expect(screen.getByText('500')).toBeInTheDocument(); | |
| // Carol + Proposal = single value 4000 -> min = 4000 | |
| expect(screen.getByText('4000')).toBeInTheDocument(); | |
| }); | |
| it('should support max aggregation, including negative values', () => { | |
| const dataWithNegative = [ | |
| ...SAMPLE_DATA, | |
| { owner: 'Dave', stage: 'Discovery', amount: -100 }, | |
| ]; | |
| render( | |
| <PivotTable | |
| schema={makeSchema({ | |
| aggregation: 'max', | |
| data: dataWithNegative, | |
| })} | |
| />, | |
| ); | |
| // Alice + Discovery = max(1000, 500) = 1000 | |
| expect(screen.getByText('1000')).toBeInTheDocument(); | |
| // Dave + Discovery = single negative value -100 -> max = -100 | |
| expect(screen.getByText('-100')).toBeInTheDocument(); | |
| }); |
| // Bob + Closed = $5,000 | ||
| expect(screen.getByText('$5,000')).toBeInTheDocument(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Missing test coverage for the columnColors feature. The PivotTableSchema includes a columnColors property to apply Tailwind color classes to column headers and cells, but no test verifies that these classes are correctly applied to the rendered elements. Consider adding a test that checks if the specified color classes appear in the DOM.
| it('should apply columnColors classes to headers and cells', () => { | |
| const schema = makeSchema({ | |
| columnColors: { | |
| Discovery: 'bg-red-100', | |
| Proposal: 'bg-blue-100', | |
| }, | |
| }); | |
| const { container } = render(<PivotTable schema={schema} />); | |
| // Headers should have the corresponding Tailwind color classes | |
| const discoveryHeader = screen.getByText('Discovery'); | |
| expect(discoveryHeader).toHaveClass('bg-red-100'); | |
| const proposalHeader = screen.getByText('Proposal'); | |
| expect(proposalHeader).toHaveClass('bg-blue-100'); | |
| // Data cells for those columns should also receive the same classes | |
| const discoveryCells = container.querySelectorAll('td.bg-red-100'); | |
| expect(discoveryCells.length).toBeGreaterThan(0); | |
| const proposalCells = container.querySelectorAll('td.bg-blue-100'); | |
| expect(proposalCells.length).toBeGreaterThan(0); | |
| }); |
Adds a PivotTable component rendering
rowField × columnField → valueFieldwith configurable aggregation, totals, formatting, and column colors. Integrates as a dashboard widget viatype='pivot'.Types (
@object-ui/types)PivotTableSchemainterface:rowField,columnField,valueField,aggregation,data,showRowTotals,showColumnTotals,format,columnColorsPivotAggregationtype:sum | count | avg | min | maxComponent (
plugin-dashboard/PivotTable.tsx)useMemo-based pivot computation: buckets raw data → aggregates per cell$,.2f)columnColorsmaps column values to Tailwind classesDashboard integration
ComponentRegistryas'pivot'DashboardRendererhandlestype='pivot'widget shorthandUsage
Tests
Roadmap
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.