From f4584ea5b62c6a0191bf26aebd0833d796162e2b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 21:03:48 +0000 Subject: [PATCH 1/6] fix(plugin-charts): a two-dimension pie/donut/funnel/treemap draws every group or refuses (WIP) Claude-Session: https://claude.ai/code/session_01Utp6CanXq6PY9nH1pZ5E5m Co-authored-by: Claude --- .../plugin-charts/src/AdvancedChartImpl.tsx | 183 +++++++++++++- ...pl.twoDimensionSingleSeries-27563.test.tsx | 233 ++++++++++++++++++ .../plugin-dashboard/src/DatasetWidget.tsx | 33 +-- ...et.twoDimensionSingleSeries-27563.test.tsx | 118 +++++++++ 4 files changed, 545 insertions(+), 22 deletions(-) create mode 100644 ui/packages/plugin-charts/src/AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx create mode 100644 ui/packages/plugin-dashboard/src/__tests__/DatasetWidget.twoDimensionSingleSeries-27563.test.tsx diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx index b1e3dd96b95..ef15a1a429a 100644 --- a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -1154,11 +1154,13 @@ function SankeyOmittedRowsNote({ shown, total, dataKey }: { shown: number; total } /** - * The series a sankey draws as FLOWS (objectstack#27460): every series except a - * comparison overlay, which is the same measure one period back and no group of - * it, so it never becomes a flow. + * The series a one-series family (sankey, treemap, pie, donut, funnel) reads as + * the GROUPS of a pivoted second dimension (objectstack#27460, + * objectstack#27563): every series except a comparison overlay, which is the + * same measure one period back and no group of it. Two or more of them mean the + * chart's one measure arrived split by a second dimension; one means it did not. */ -const sankeyFlowSeries = (series: readonly NormalizedSeries[]): NormalizedSeries[] => +const groupSeries = (series: readonly NormalizedSeries[]): NormalizedSeries[] => series.filter((s) => s.variant !== 'comparison'); /** One drawn flow of a two-level sankey, and the cell it was drawn from. */ @@ -1240,6 +1242,64 @@ function buildSankeyCellFlows( return { nodes, links, omitted }; } +/** One leaf of a two-level treemap: a (category, group) cell, and the row and series it was drawn from. */ +interface TreemapCellLeaf { + name: string; + size: number; + fill: string; + /** The `data` row the cell belongs to — what a click drills to, with `seriesKey`. */ + rowIndex: number; + /** The series (the second dimension's group) the cell is. */ + seriesKey: string; +} + +/** + * A TWO-LEVEL treemap: each row's category is a parent, and each group it + * carries is a leaf inside it (objectstack#27563). + * + * The treemap counterpart of {@link buildSankeyCellFlows}, for the same input: + * two or more groups reaching the arm are the groups of its one measure, split + * by a pivoted second dimension. Nesting is native to Recharts' `Treemap` (a + * parent's area is the sum of its leaves), so every cell is drawn; the arm used + * to read `series[0]` alone and drew the first group's cells as the whole chart. + * + * A cell the row does not CARRY is no leaf and no omission (the pivot writes a + * column only for a combination the dataset answered). A carried cell that + * cannot be sized (not a finite number above zero, the `countSizableRows` test) + * is left out and counted per group, so the chart says which group lost it. + * A leaf is named `category / group`, the title the dashboard's drill gives the + * same cell, because a parent's own name is painted over by its leaves; the + * leaves of one category share its colour. + */ +function buildTreemapGroups( + data: ReadonlyArray>, + xAxisKey: string, + groups: readonly NormalizedSeries[], + palette: readonly string[], +): { nodes: Array<{ name: string; fill: string; children: TreemapCellLeaf[] }>; carried: number; omitted: SankeyOmittedColumn[] } { + const isSizable = (v: unknown) => Number.isFinite(Number(v)) && Number(v) > 0; + const carries = (row: Record | null | undefined, key: string) => + row != null && Object.prototype.hasOwnProperty.call(row, key); + const nodes = data.flatMap((row, rowIndex) => { + const category = String(row?.[xAxisKey] ?? ''); + const fill = resolveColor(palette[rowIndex % palette.length]); + const children = groups.flatMap((s) => + carries(row, s.dataKey) && isSizable(row[s.dataKey]) + ? [{ name: `${category} / ${String(s.label ?? s.dataKey)}`, size: Number(row[s.dataKey]), fill, rowIndex, seriesKey: s.dataKey }] + : [], + ); + return children.length > 0 ? [{ name: category, fill, children }] : []; + }); + let carried = 0; + const omitted = groups.flatMap((s) => { + const total = data.filter((row) => carries(row, s.dataKey)).length; + const shown = data.filter((row) => carries(row, s.dataKey) && isSizable(row[s.dataKey])).length; + carried += total; + return shown < total ? [{ label: String(s.label ?? s.dataKey), shown, total }] : []; + }); + return { nodes, carried, omitted }; +} + /** * Renders the spec's `ChartConfig.title` / `.subtitle` above the plot. * @@ -1385,12 +1445,60 @@ function countSizableRows(rows: unknown[], dataKey: string): { sizable: number; * exist. That is the empty-RESULT question (objectui#7130), answered upstream in * `ObjectChart` where the query outcome is known — so every no-rows tile is left * byte-for-byte as it was. + * + * A two-level treemap (objectstack#27563) tests every group's column, so it + * hands a list and the sentence names each one, as `no-positive-flow` does. */ -function MagnitudeRefusal({ dataKey, className }: { dataKey: string; className?: string }) { +function MagnitudeRefusal({ dataKey, className }: { dataKey: string | readonly string[]; className?: string }) { + const keys = typeof dataKey === 'string' ? [dataKey] : dataKey; return ( This chart has nothing to size: no row's{' '} - {dataKey} is above zero. + {keys.map((key, i) => ( + + {i > 0 ? ' / ' : null} + {key} + + ))}{' '} + is above zero. + + ); +} + +/** + * The refusal a ONE-series family (pie, donut, funnel) renders when it is + * handed two or more groups (objectstack#27563). + * + * Those groups are almost always a second dimension the dataset pivoted into + * one series per value. A pie's angle is one share of one whole and a funnel's + * width one total per stage, so neither has a place for a second series, and + * drawing the first one alone is a partial chart that looks complete: every + * other group silently dropped. So the chart draws nothing and names every + * group, which stays true when the series are measures instead of groups. + * (The treemap nests them and the sankey flows them; both draw every group.) + * + * Called only with rows to draw: handed NO rows, the chart is the empty-result + * tile it always was, as `MagnitudeRefusal`'s callers gate on `total > 0`. + */ +function OneSeriesFamilyRefusal({ + family, + groups, + className, +}: { + family: string; + groups: readonly NormalizedSeries[]; + className?: string; +}) { + return ( + + A {family} draws one series, and this chart carries {groups.length}:{' '} + {groups.map((s, i) => ( + + {i > 0 ? ', ' : null} + {String(s.label ?? s.dataKey)} + + ))} + . Drawing only the first would hide the others, so none is drawn. ); } @@ -2137,6 +2245,27 @@ function AdvancedChartImplInner({ }); }, [onChartClick, data, xAxisKey, series]); + // A two-level treemap (objectstack#27563) drills on a LEAF, which is exactly + // one cell: Recharts reports only leaf clicks, and each leaf carries its row + // and series, the `{ category, series }` pair the drill lookup resolves to one + // dataset row — the two-level sankey's flow click, for a leaf. + const handleTreemapCellClick = React.useCallback((node: any) => { + if (!onChartClick) return; + const leaf = node as Partial | undefined; + const row = typeof leaf?.rowIndex === 'number' ? data[leaf.rowIndex] : undefined; + const key = leaf?.seriesKey; + if (!row || typeof key !== 'string') return; + const cat = row[xAxisKey]; + onChartClick({ + category: cat != null ? String(cat) : undefined, + categoryId: chartRowBucketId(row), + series: key, + seriesLabel: seriesLabelForKey(key, series), + value: typeof row[key] === 'number' ? row[key] : undefined, + }); + }, [onChartClick, data, xAxisKey, series]); + + const treemapCellClickProps = onChartClick ? { onClick: handleTreemapCellClick, style: { cursor: 'pointer' as const } } : {}; const sankeyClickProps = onChartClick ? { onClick: handleSankeyClick, style: { cursor: 'pointer' as const } } : {}; const sankeyFlowClickProps = onChartClick ? { onClick: handleSankeyFlowClick, style: { cursor: 'pointer' as const } } : {}; @@ -2617,6 +2746,12 @@ function AdvancedChartImplInner({ // Pie and Donut charts if (chartType === 'pie' || chartType === 'donut') { + // objectstack#27563 — two or more groups have no ring to go on; see + // `OneSeriesFamilyRefusal`. Never the first group's slices alone. + const pieGroups = groupSeries(series); + if (pieGroups.length >= 2 && data.length > 0) { + return ; + } const innerRadius = chartType === 'donut' ? '52%' : 0; const palette = getPalette(); // objectui#7147 — see `countSizableRows`. A slice's angle is its share of @@ -2690,6 +2825,11 @@ function AdvancedChartImplInner({ // Funnel chart — uses recharts FunnelChart (single series only) if (chartType === 'funnel') { + // objectstack#27563 — a funnel stage is one total; see `OneSeriesFamilyRefusal`. + const funnelGroups = groupSeries(series); + if (funnelGroups.length >= 2 && data.length > 0) { + return ; + } const dataKey = series[0]?.dataKey || 'value'; const palette = getPalette(); // objectui#7147 — see `countSizableRows`. Recharts derives each segment's @@ -2782,6 +2922,35 @@ function AdvancedChartImplInner({ // chart root (no wrapping cartesian chart); a custom content paints each // leaf with a palette color + label. if (chartType === 'treemap') { + // Two or more groups are a pivoted second dimension, drawn as a second + // level of leaves (objectstack#27563, `buildTreemapGroups`). One group + // keeps the one-level treemap below. + const treemapGroups = groupSeries(series); + if (treemapGroups.length >= 2 && data.length > 0) { + const cells = buildTreemapGroups(data, xAxisKey, treemapGroups, getPalette()); + if (cells.nodes.length === 0 && cells.carried > 0) { + return String(s.label ?? s.dataKey))} className={className} />; + } + return ( + 0 ? ( + <> + {cells.omitted.map((c) => ( + {unsizedRowsNote(c.shown, c.total, c.label)} + ))} + + ) : null + } + > + + } {...treemapCellClickProps}> + + + + + ); + } const dataKey = series[0]?.dataKey || 'value'; const palette = getPalette(); // objectui#7147 — see `countSizableRows`. A treemap's leaf area IS its @@ -2820,7 +2989,7 @@ function AdvancedChartImplInner({ // Two or more flow series are the groups of a pivoted second dimension, // drawn as a second column of nodes (objectstack#27460, // `buildSankeyCellFlows`). One series keeps the one-level flow below. - const flowSeries = sankeyFlowSeries(series); + const flowSeries = groupSeries(series); if (flowSeries.length >= 2) { const flows = buildSankeyCellFlows(data, xAxisKey, flowSeries, getPalette()); if (flows.links.length === 0) { diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx new file mode 100644 index 00000000000..5bb2f4cd70b --- /dev/null +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx @@ -0,0 +1,233 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * A pie, donut, funnel or treemap with TWO dimensions draws every group, or + * refuses on the tile; it never draws the first group alone (objectstack#27563). + * + * `buildChartSeries` pivots a second dimension into series when the chart + * carries one measure: one row per first-dimension value, one series per + * second-dimension value. These four arms read `series[0]` alone, so a + * `stage × owner` amount drew only `Ann`'s amounts, and `Bo` and `Cy` were + * dropped with nothing on the chart saying so. The pivot is built here with + * the same helper the dashboard widget and the inline chart call, so the + * fixture is the shape the arms are really handed. + * + * - treemap draws every cell, nested under its first-dimension value; + * - pie, donut and funnel have no geometry for a second series, so they refuse + * on the tile and name every group, instead of drawing the first one. + * + * The marks are read off the props each recharts element was rendered with, + * not off jsdom pixels, which are not deterministic for a pie. + */ +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import { buildChartSeries } from '@object-ui/core'; + +/** Every `data` prop each recharts element was rendered with, newest last. */ +const drawn = { + pie: [] as Array<{ data: Array>; dataKey: string }>, + funnel: [] as Array<{ data: Array>; dataKey: string }>, + treemap: [] as Array>>, + treemapProps: [] as Array>, +}; + +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + Pie: (props: any) => { + drawn.pie.push({ data: props.data, dataKey: props.dataKey }); + return ; + }, + Funnel: (props: any) => { + drawn.funnel.push({ data: props.data, dataKey: props.dataKey }); + return ; + }, + Treemap: (props: any) => { + drawn.treemap.push(props.data); + drawn.treemapProps.push(props); + return ; + }, + }; +}); + +import AdvancedChartImpl from './AdvancedChartImpl'; + +afterEach(() => { + cleanup(); + drawn.pie.length = 0; + drawn.funnel.length = 0; + drawn.treemap.length = 0; + drawn.treemapProps.length = 0; +}); + +/** The dataset's answer: one row per (stage, owner) combination it found. */ +const FLAT = [ + { stage: 'Prospect', owner: 'Ann', amount: 100 }, + { stage: 'Prospect', owner: 'Bo', amount: 50 }, + { stage: 'Proposal', owner: 'Ann', amount: 80 }, + { stage: 'Proposal', owner: 'Cy', amount: 40 }, + { stage: 'Won', owner: 'Bo', amount: 120 }, +]; + +const pivot = (rows: Array>) => + buildChartSeries(rows, ['stage', 'owner'], ['amount']); + +const oneDimension = (rows: Array>) => + buildChartSeries(rows, ['stage'], ['amount']); + +const renderChart = ( + chartType: string, + chart: { data: Array>; xAxisKey: string; series: unknown[] }, + extra: Record = {}, +) => + render( + , + ); + +/** The ONE-dimension aggregate of the same records: the regression control's input. */ +const BY_STAGE = [ + { stage: 'Prospect', amount: 150 }, + { stage: 'Proposal', amount: 120 }, + { stage: 'Won', amount: 120 }, +]; + +/** The drawn treemap as `parent: leaf=size, …` per first-dimension value. */ +const treemapCells = () => + drawn.treemap[drawn.treemap.length - 1].map( + (n) => `${n.name}: ${(n.children ?? []).map((c: any) => `${c.name}=${c.size}`).join(', ')}`, + ); + +const refusalOf = (container: HTMLElement) => container.querySelector('[data-chart-error]'); +const noteTexts = (container: HTMLElement) => + [...container.querySelectorAll('[data-chart-note]')].map((n) => n.textContent); + +describe('objectstack#27563 — a two-dimension single-series chart draws every group or refuses', () => { + it('the fixture is a real pivot: three first-dimension rows, three series', () => { + const { data, series } = pivot(FLAT); + expect(data.map((r) => r.stage)).toEqual(['Prospect', 'Proposal', 'Won']); + expect(series.map((s) => s.label)).toEqual(['Ann', 'Bo', 'Cy']); + }); + + describe('treemap draws every cell, nested under its first-dimension value', () => { + it('one leaf per (stage, owner) cell the dataset answered, sized by the measure', () => { + const { container } = renderChart('treemap', pivot(FLAT)); + expect(treemapCells()).toEqual([ + 'Prospect: Prospect / Ann=100, Prospect / Bo=50', + 'Proposal: Proposal / Ann=80, Proposal / Cy=40', + 'Won: Won / Bo=120', + ]); + // A combination with no records (Prospect × Cy, …) is no leaf and no omission. + expect(noteTexts(container)).toEqual([]); + expect(refusalOf(container)).toBeNull(); + // Recharts laid out and painted all five leaves. + expect(container.querySelectorAll('.recharts-treemap-depth-2')).toHaveLength(5); + }); + + it('the leaves of one first-dimension value share its colour; the values differ', () => { + renderChart('treemap', pivot(FLAT)); + const parents = drawn.treemap[drawn.treemap.length - 1]; + for (const p of parents) { + for (const leaf of p.children) expect(leaf.fill).toBe(p.fill); + } + expect(new Set(parents.map((p) => p.fill)).size).toBe(parents.length); + }); + + it('a carried cell that is not above zero is not drawn, and the chart says which group lost it', () => { + const rows = FLAT.map((r) => (r.stage === 'Won' && r.owner === 'Bo' ? { ...r, amount: -5 } : r)); + const { container } = renderChart('treemap', pivot(rows)); + expect(treemapCells()).toEqual([ + 'Prospect: Prospect / Ann=100, Prospect / Bo=50', + 'Proposal: Proposal / Ann=80, Proposal / Cy=40', + ]); + expect(noteTexts(container)).toEqual([ + '1 of 2 rows has no Bo above zero — this chart sizes by value, so that row is not drawn to scale.', + ]); + }); + + it('no cell above zero is the magnitude refusal, naming every group it tested', () => { + const rows = FLAT.map((r) => ({ ...r, amount: 0 })); + const { container } = renderChart('treemap', pivot(rows)); + expect(refusalOf(container)?.getAttribute('data-chart-error')).toBe('no-positive-magnitude'); + expect([...refusalOf(container)!.querySelectorAll('code')].map((c) => c.textContent)).toEqual(['Ann', 'Bo', 'Cy']); + expect(drawn.treemap).toHaveLength(0); + }); + + it('clicking a leaf drills to its cell: the first-dimension value and the group', () => { + const onChartClick = vi.fn(); + renderChart('treemap', pivot(FLAT), { onChartClick }); + const leaf = { ...drawn.treemap[drawn.treemap.length - 1][1].children[1], depth: 2 }; + drawn.treemapProps[drawn.treemapProps.length - 1].onClick(leaf); + expect(onChartClick).toHaveBeenCalledWith( + expect.objectContaining({ category: 'Proposal', series: 'Cy', seriesLabel: 'Cy', value: 40 }), + ); + }); + + it('REGRESSION: a one-dimension treemap draws exactly as before, one flat leaf per row', () => { + const { container } = renderChart('treemap', oneDimension(BY_STAGE)); + expect(drawn.treemap[drawn.treemap.length - 1].map((n) => `${n.name}=${n.size}`)).toEqual([ + 'Prospect=150', + 'Proposal=120', + 'Won=120', + ]); + expect(drawn.treemap[drawn.treemap.length - 1].every((n) => n.children === undefined)).toBe(true); + expect(refusalOf(container)).toBeNull(); + }); + + it('REGRESSION: a comparison overlay is not a group — a compared treemap stays one level', () => { + const { data, xAxisKey, series } = oneDimension(BY_STAGE); + renderChart('treemap', { + data, + xAxisKey, + series: [ + { ...series[0], variant: 'current' }, + { dataKey: 'amount__compare', label: 'Amount · previous', variant: 'comparison' }, + ], + }); + expect(drawn.treemap[drawn.treemap.length - 1].map((n) => n.name)).toEqual(['Prospect', 'Proposal', 'Won']); + }); + }); + + describe.each(['pie', 'donut', 'funnel'] as const)('%s refuses on the tile and names every group', (family) => { + const marks = () => (family === 'funnel' ? drawn.funnel : drawn.pie); + + it('draws no mark of the first group alone', () => { + const { container } = renderChart(family, pivot(FLAT)); + expect(marks()).toHaveLength(0); + expect(container.querySelector('.recharts-surface')).toBeNull(); + }); + + it('the refusal names the family and every group', () => { + const { container } = renderChart(family, pivot(FLAT)); + const refusal = refusalOf(container)!; + expect(refusal.getAttribute('data-chart-error')).toBe('one-series-family'); + expect(refusal.getAttribute('role')).toBe('status'); + expect([...refusal.querySelectorAll('code')].map((c) => c.textContent)).toEqual(['Ann', 'Bo', 'Cy']); + expect(refusal.textContent).toContain(`A ${family} draws one series, and this chart carries 3:`); + }); + + it(`REGRESSION: a one-dimension ${family} draws exactly as before, one mark per row`, () => { + const { container } = renderChart(family, oneDimension(BY_STAGE)); + expect(refusalOf(container)).toBeNull(); + const last = marks()[marks().length - 1]; + expect(last.dataKey).toBe('amount'); + expect(last.data.map((r) => r.stage)).toHaveLength(3); + }); + + it('REGRESSION: no rows is the empty chart it always was, not a refusal', () => { + const { container } = renderChart(family, { data: [], xAxisKey: 'stage', series: pivot(FLAT).series }); + expect(refusalOf(container)).toBeNull(); + }); + }); +}); diff --git a/ui/packages/plugin-dashboard/src/DatasetWidget.tsx b/ui/packages/plugin-dashboard/src/DatasetWidget.tsx index eb6af87e4d6..b0b7e57e371 100644 --- a/ui/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/ui/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -472,14 +472,14 @@ const MEASURE_AXIS_CHART_FAMILIES: ReadonlySet = new Set(DASHBOARD_WIDGE /** * The chart families that draw ONE series, whatever the widget declares * (objectui#11417). The shared chart renderer (`AdvancedChartImpl` in - * `@object-ui/plugin-charts`) binds `series[0]` on its `pie` / `donut`, - * `funnel` and `treemap` arms and reads no other series there, so a chart of - * one of these families WITH a dimension draws its first measure and drops - * every other one, although this widget hands each measure on as a series. - * Its `sankey` arm reads the series past the first as the groups of a pivoted - * second dimension (objectstack#27460), so a sankey is handed only the measure - * it renders (see `chartSeries`). (Without a dimension these families take the - * tile, `isMetric`.) + * `@object-ui/plugin-charts`) draws one measure on its `pie` / `donut`, + * `funnel`, `treemap` and `sankey` arms, and reads the series past the first + * as the groups of a pivoted second dimension (objectstack#27460, + * objectstack#27563): the sankey and the treemap draw every group, the pie, + * donut and funnel refuse on the tile. So a chart of one of these families + * WITH a dimension is handed only the measure it renders (see `chartSeries`), + * and draws its first measure while every other one is dropped. (Without a + * dimension these families take the tile, `isMetric`.) * * Read off the family a widget renders AS (`CHART_TYPE_MAP`), so a `pyramid` * widget, which renders as a `funnel`, is in it too. @@ -716,8 +716,9 @@ export function DatasetWidget({ widget, dataSource, heading, onForbidden }: { const chartType = CHART_TYPE_MAP[widgetType] ?? 'bar'; // A chart of a single-series family draws its first measure only, with a // dimension too (`SINGLE_SERIES_CHART_FAMILIES`, objectui#11417). The chart - // branch still hands every measure on as a series: what is drawn does not - // change here, only what this widget says it renders. + // branch hands such a chart that first measure alone (`chartSeries`), because + // the renderer reads a second series there as a group of a pivoted second + // dimension (objectstack#27563). const drawsFirstMeasureOnly = !isMetric && !isTable && SINGLE_SERIES_CHART_FAMILIES.has(chartType); // ── What this widget RENDERS of what it declares (objectui#8894) ───────── // Settled here, by the branch decision just above, and read by both the tile @@ -730,7 +731,7 @@ export function DatasetWidget({ widget, dataSource, heading, onForbidden }: { // // "Renders" is what reaches the screen. For a chart that is decided in the // shared chart renderer, which this widget hands every measure to as a - // series; the one family of arms there that draws a single series is + // series but on the single-series families; that one family of arms is // stated here as `SINGLE_SERIES_CHART_FAMILIES`, with the pin that ties the // two named in its docblock. const renderedMeasures = isMetric || drawsFirstMeasureOnly ? values.slice(0, 1) : values; @@ -2097,13 +2098,15 @@ export function DatasetWidget({ widget, dataSource, heading, onForbidden }: { const comparedChartSeries = comparisonSeries.length > 0 ? [...series.map((s) => ({ ...s, variant: 'current' as const })), ...comparisonSeries] : series; - // objectstack#27460 — a sankey draws every series past its first as a group - // of a pivoted second dimension, one flow per cell. A sankey that declares + // objectstack#27460, objectstack#27563 — a single-series family reads every + // series past its first as a group of a pivoted second dimension: a sankey + // draws one flow per cell, a treemap one leaf per cell, and a pie, donut or + // funnel refuses on the tile. A widget of these families that declares // several measures (refused by the spec's single-series arm, so only a stored // document reaches here) is handed the one measure it renders, so its other - // measures are not drawn as groups and the dropped-measure warning above + // measures are not read as groups and the dropped-measure warning above // stays true. A pivot carries one measure, so nothing is dropped from it. - const chartSeries = chartType === 'sankey' + const chartSeries = SINGLE_SERIES_CHART_FAMILIES.has(chartType) ? comparedChartSeries.filter((s) => !droppedMeasures.includes(s.dataKey)) : comparedChartSeries; diff --git a/ui/packages/plugin-dashboard/src/__tests__/DatasetWidget.twoDimensionSingleSeries-27563.test.tsx b/ui/packages/plugin-dashboard/src/__tests__/DatasetWidget.twoDimensionSingleSeries-27563.test.tsx new file mode 100644 index 00000000000..0568e4402b5 --- /dev/null +++ b/ui/packages/plugin-dashboard/src/__tests__/DatasetWidget.twoDimensionSingleSeries-27563.test.tsx @@ -0,0 +1,118 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#27563 — the card's own widgets, through the real chain: a pie, + * donut, funnel or treemap with `dimensions: ['stage', 'owner']` and one + * amount measure. + * + * The widget pivots the second dimension into one series per owner, and these + * arms drew `series[0]` only: the first owner's amounts, with the other owners + * dropped and nothing saying so. The cells are pinned beside the renderer + * (`AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx`); this file pins + * that the dashboard hands every group on, that the treemap draws every cell + * and that the pie, donut and funnel name every group in a refusal on the tile, + * and that a stored widget declaring two measures still draws its first one. + * + * The chain is `DatasetWidget` → `SchemaRenderer` → the registry's `chart` → + * `AdvancedChartImpl`, with the scoped `getBoundingClientRect` stub of + * `DatasetWidget.singleSeriesMeasures-11417.test.tsx`, whose header says why + * `vi.mock('recharts')` is not available in this package. + */ + +import { describe, it, expect, vi, afterEach, beforeAll, afterAll } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +// Registers `chart`, reached through `ChartRenderer`'s `React.lazy` factory as +// in production (objectui#4529). +import '@object-ui/plugin-charts'; +import { DatasetWidget } from '../DatasetWidget'; + +const PLOT_BOX = { width: 480, height: 320 } as const; +const originalGetBoundingClientRect = HTMLElement.prototype.getBoundingClientRect; + +beforeAll(() => { + HTMLElement.prototype.getBoundingClientRect = function (this: HTMLElement) { + if (this.classList?.contains('recharts-responsive-container')) { + return { + ...PLOT_BOX, + top: 0, + left: 0, + right: PLOT_BOX.width, + bottom: PLOT_BOX.height, + x: 0, + y: 0, + toJSON() {}, + } as DOMRect; + } + return originalGetBoundingClientRect.call(this); + }; +}); + +afterAll(() => { + HTMLElement.prototype.getBoundingClientRect = originalGetBoundingClientRect; +}); + +afterEach(cleanup); + +const ROWS = [ + { stage: 'Prospect', owner: 'Ann', amount: 100, cost: 10 }, + { stage: 'Prospect', owner: 'Bo', amount: 50, cost: 5 }, + { stage: 'Proposal', owner: 'Ann', amount: 80, cost: 8 }, + { stage: 'Proposal', owner: 'Cy', amount: 40, cost: 4 }, + { stage: 'Won', owner: 'Bo', amount: 120, cost: 12 }, +]; + +const renderWidget = async (type: string, dimensions: string[], values: string[] = ['amount']) => { + const src = { queryDataset: vi.fn(async () => ({ rows: ROWS })) }; + const view = render( + , + ); + await waitFor( + () => expect(view.container.querySelector('.recharts-surface, [data-chart-error]')).not.toBeNull(), + { timeout: 15000 }, + ); + return view; +}; + +describe('objectstack#27563 — a two-dimension single-series widget draws every group or refuses', () => { + it('treemap: draws one leaf per answered (stage, owner) pair, every owner included', async () => { + const { container } = await renderWidget('treemap', ['stage', 'owner']); + expect(container.querySelector('[data-chart-note], [data-chart-error]')).toBeNull(); + expect(container.querySelectorAll('.recharts-treemap-depth-2')).toHaveLength(ROWS.length); + }); + + it.each(['pie', 'donut', 'funnel'])('%s: refuses on the tile and names every owner', async (type) => { + const { container } = await renderWidget(type, ['stage', 'owner']); + expect(container.querySelector('.recharts-surface')).toBeNull(); + const refusal = container.querySelector('[data-chart-error]')!; + expect(refusal.getAttribute('data-chart-error')).toBe('one-series-family'); + expect([...refusal.querySelectorAll('code')].map((c) => c.textContent)).toEqual(['Ann', 'Bo', 'Cy']); + }); + + it.each(['pie', 'donut', 'funnel', 'treemap'])( + 'CONTROL: a one-dimension %s draws, with no refusal and no note', + async (type) => { + const { container } = await renderWidget(type, ['stage']); + expect(container.querySelector('.recharts-surface')).not.toBeNull(); + expect(container.querySelector('[data-chart-note], [data-chart-error]')).toBeNull(); + }, + ); + + it.each(['pie', 'donut', 'funnel', 'treemap'])( + 'CONTROL: a stored one-dimension %s with two measures still draws its first measure, not a group refusal', + async (type) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const { container } = await renderWidget(type, ['stage'], ['amount', 'cost']); + expect(container.querySelector('.recharts-surface')).not.toBeNull(); + expect(container.querySelector('[data-chart-error]')).toBeNull(); + // A treemap would nest the second measure under each stage if it were handed it. + expect(container.querySelectorAll('.recharts-treemap-depth-2')).toHaveLength(0); + } finally { + warn.mockRestore(); + } + }, + ); +}); From 44fc6496aa7862dca1beb03ec9a2897bb350b356 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 21:07:30 +0000 Subject: [PATCH 2/6] fix(plugin-charts): type the treemap leaf as an alias; add the changeset (WIP) Claude-Session: https://claude.ai/code/session_01Utp6CanXq6PY9nH1pZ5E5m Co-authored-by: Claude --- .changeset/27563-two-dimension-single-series.md | 15 +++++++++++++++ .../plugin-charts/src/AdvancedChartImpl.tsx | 10 +++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 .changeset/27563-two-dimension-single-series.md diff --git a/.changeset/27563-two-dimension-single-series.md b/.changeset/27563-two-dimension-single-series.md new file mode 100644 index 00000000000..90ef490de23 --- /dev/null +++ b/.changeset/27563-two-dimension-single-series.md @@ -0,0 +1,15 @@ +--- +'@object-ui/plugin-charts': patch +'@object-ui/plugin-dashboard': patch +--- + +fix(plugin-charts): a pie, donut, funnel or treemap with two dimensions draws every second-dimension group or refuses on the tile, instead of silently drawing the first group only + +Clause-②: no +Metadata-protocol: none + +A dashboard widget (or a dataset-bound chart) of type `pie`, `donut`, `funnel` or `treemap` with `dimensions: ['stage', 'owner']` and one measure used to draw only the first `owner` value's amounts; every other owner was dropped and nothing on the chart said so. The second dimension reaches the renderer pivoted into one series per value, and these arms read the first series only. + +- **Treemap draws every cell.** Two or more series on a treemap are now drawn as two levels: each first-dimension value is a parent, and each second-dimension value it has records for is a leaf inside it, sized by the measure and named `stage / owner`. A cell that is not above zero is left out, and the chart prints the existing "not drawn to scale" note once per second-dimension value that lost a cell; a chart with no cell above zero shows the existing "nothing to size" refusal, naming every value it tested. Clicking a leaf drills to its cell. +- **Pie, donut and funnel refuse on the tile.** A pie slice is one share of one whole and a funnel stage one total, so neither has a place for a second series. Handed two or more, they now draw nothing and show a refusal (`data-chart-error="one-series-family"`) that names every group, instead of the first group's chart. +- **Unchanged:** a chart with one dimension draws exactly as before on all four types, a comparison overlay (`compareTo`) is never read as a group, and a chart with no rows is the empty chart it was. A stored dashboard widget of these types that declares several measures (refused at publish by the spec) is handed only its first measure, as before, so its other measures are not read as groups. diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx index ef15a1a429a..707fac5e26d 100644 --- a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -1242,8 +1242,12 @@ function buildSankeyCellFlows( return { nodes, links, omitted }; } -/** One leaf of a two-level treemap: a (category, group) cell, and the row and series it was drawn from. */ -interface TreemapCellLeaf { +/** + * One leaf of a two-level treemap: a (category, group) cell, and the row and + * series it was drawn from. A type alias, not an interface: Recharts' treemap + * datum is an index-signature record, which only an alias is assignable to. + */ +type TreemapCellLeaf = { name: string; size: number; fill: string; @@ -1251,7 +1255,7 @@ interface TreemapCellLeaf { rowIndex: number; /** The series (the second dimension's group) the cell is. */ seriesKey: string; -} +}; /** * A TWO-LEVEL treemap: each row's category is a parent, and each group it From 5d26a45372383bea6cc7952079d68d84efdeeccc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 21:10:30 +0000 Subject: [PATCH 3/6] refactor(plugin-charts): type the two-level treemap's rows and leaf click as unknown, not any Claude-Session: https://claude.ai/code/session_01Utp6CanXq6PY9nH1pZ5E5m Co-authored-by: Claude --- ui/packages/plugin-charts/src/AdvancedChartImpl.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx index 707fac5e26d..2d895c87537 100644 --- a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -1276,13 +1276,13 @@ type TreemapCellLeaf = { * leaves of one category share its colour. */ function buildTreemapGroups( - data: ReadonlyArray>, + data: ReadonlyArray>, xAxisKey: string, groups: readonly NormalizedSeries[], palette: readonly string[], ): { nodes: Array<{ name: string; fill: string; children: TreemapCellLeaf[] }>; carried: number; omitted: SankeyOmittedColumn[] } { const isSizable = (v: unknown) => Number.isFinite(Number(v)) && Number(v) > 0; - const carries = (row: Record | null | undefined, key: string) => + const carries = (row: Record | null | undefined, key: string) => row != null && Object.prototype.hasOwnProperty.call(row, key); const nodes = data.flatMap((row, rowIndex) => { const category = String(row?.[xAxisKey] ?? ''); @@ -2253,7 +2253,7 @@ function AdvancedChartImplInner({ // one cell: Recharts reports only leaf clicks, and each leaf carries its row // and series, the `{ category, series }` pair the drill lookup resolves to one // dataset row — the two-level sankey's flow click, for a leaf. - const handleTreemapCellClick = React.useCallback((node: any) => { + const handleTreemapCellClick = React.useCallback((node: unknown) => { if (!onChartClick) return; const leaf = node as Partial | undefined; const row = typeof leaf?.rowIndex === 'number' ? data[leaf.rowIndex] : undefined; From cd9dfd3cde02a766041c442933afc3eb14e112f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 21:21:10 +0000 Subject: [PATCH 4/6] test(plugin-charts): type the 27563 renderer fixture; the 10691 pie control settles on the one-series refusal Claude-Session: https://claude.ai/code/session_01Utp6CanXq6PY9nH1pZ5E5m Co-authored-by: Claude --- ...vancedChartImpl.twoDimensionSingleSeries-27563.test.tsx | 4 ++-- .../src/ChartRenderer.yAxisUndrawn-10691.test.tsx | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx index 5bb2f4cd70b..cab81c6c67d 100644 --- a/ui/packages/plugin-charts/src/AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.twoDimensionSingleSeries-27563.test.tsx @@ -82,14 +82,14 @@ const oneDimension = (rows: Array>) => const renderChart = ( chartType: string, - chart: { data: Array>; xAxisKey: string; series: unknown[] }, + chart: { data: Array>; xAxisKey: string | undefined; series: unknown[] }, extra: Record = {}, ) => render( ) { schema={{ type: 'chart', data: ROWS, xAxis: { field: 'month' }, isAnimationActive: false, ...schema } as never} />, ); - await waitFor(() => expect(container.querySelector('svg')).toBeTruthy()); + // Settled on the drawn plot, or on the renderer's own refusal (a pie handed + // several series refuses, objectstack#27563). + await waitFor(() => expect(container.querySelector('svg, [data-chart-error]')).toBeTruthy()); return container; } @@ -167,6 +169,9 @@ describe('objectui#10691 — a `yAxis` entry after the second is drawn on no axi it('(control) pie, three entries: no note', async () => { const c = await draw({ chartType: 'pie', yAxis: THREE }); expect(noteKinds(c)).toEqual([]); + // A pie draws one series, so three derived series are named in its + // one-series refusal rather than the first drawn alone (objectstack#27563). + expect(c.querySelector('[data-chart-error]')?.getAttribute('data-chart-error')).toBe('one-series-family'); }); // ── One answer: the normalizer binds from the placement the note reads ── From bd3c2bff9a60bf8938d4fd255d1446a0915f1242 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 21:24:51 +0000 Subject: [PATCH 5/6] docs(changeset): name the multi-measure charts outside a dashboard that now refuse or nest Claude-Session: https://claude.ai/code/session_01Utp6CanXq6PY9nH1pZ5E5m Co-authored-by: Claude --- .changeset/27563-two-dimension-single-series.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/27563-two-dimension-single-series.md b/.changeset/27563-two-dimension-single-series.md index 90ef490de23..645ba85142f 100644 --- a/.changeset/27563-two-dimension-single-series.md +++ b/.changeset/27563-two-dimension-single-series.md @@ -12,4 +12,5 @@ A dashboard widget (or a dataset-bound chart) of type `pie`, `donut`, `funnel` o - **Treemap draws every cell.** Two or more series on a treemap are now drawn as two levels: each first-dimension value is a parent, and each second-dimension value it has records for is a leaf inside it, sized by the measure and named `stage / owner`. A cell that is not above zero is left out, and the chart prints the existing "not drawn to scale" note once per second-dimension value that lost a cell; a chart with no cell above zero shows the existing "nothing to size" refusal, naming every value it tested. Clicking a leaf drills to its cell. - **Pie, donut and funnel refuse on the tile.** A pie slice is one share of one whole and a funnel stage one total, so neither has a place for a second series. Handed two or more, they now draw nothing and show a refusal (`data-chart-error="one-series-family"`) that names every group, instead of the first group's chart. +- **Several measures outside a dashboard.** The renderer cannot tell a second-dimension group from a second measure, so a pie, donut or funnel chart block that authors a `yAxis` list, or a page chart with one dimension and two measures, now shows the same refusal naming every measure instead of drawing the first one alone, and such a treemap nests the measures under each category. - **Unchanged:** a chart with one dimension draws exactly as before on all four types, a comparison overlay (`compareTo`) is never read as a group, and a chart with no rows is the empty chart it was. A stored dashboard widget of these types that declares several measures (refused at publish by the spec) is handed only its first measure, as before, so its other measures are not read as groups. From d12c72c818c69ea25169f241868f87998e3ad941 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 21:25:03 +0000 Subject: [PATCH 6/6] docs(changeset): the unchanged case is one dimension and one measure Claude-Session: https://claude.ai/code/session_01Utp6CanXq6PY9nH1pZ5E5m Co-authored-by: Claude --- .changeset/27563-two-dimension-single-series.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/27563-two-dimension-single-series.md b/.changeset/27563-two-dimension-single-series.md index 645ba85142f..cb2d1b7d956 100644 --- a/.changeset/27563-two-dimension-single-series.md +++ b/.changeset/27563-two-dimension-single-series.md @@ -13,4 +13,4 @@ A dashboard widget (or a dataset-bound chart) of type `pie`, `donut`, `funnel` o - **Treemap draws every cell.** Two or more series on a treemap are now drawn as two levels: each first-dimension value is a parent, and each second-dimension value it has records for is a leaf inside it, sized by the measure and named `stage / owner`. A cell that is not above zero is left out, and the chart prints the existing "not drawn to scale" note once per second-dimension value that lost a cell; a chart with no cell above zero shows the existing "nothing to size" refusal, naming every value it tested. Clicking a leaf drills to its cell. - **Pie, donut and funnel refuse on the tile.** A pie slice is one share of one whole and a funnel stage one total, so neither has a place for a second series. Handed two or more, they now draw nothing and show a refusal (`data-chart-error="one-series-family"`) that names every group, instead of the first group's chart. - **Several measures outside a dashboard.** The renderer cannot tell a second-dimension group from a second measure, so a pie, donut or funnel chart block that authors a `yAxis` list, or a page chart with one dimension and two measures, now shows the same refusal naming every measure instead of drawing the first one alone, and such a treemap nests the measures under each category. -- **Unchanged:** a chart with one dimension draws exactly as before on all four types, a comparison overlay (`compareTo`) is never read as a group, and a chart with no rows is the empty chart it was. A stored dashboard widget of these types that declares several measures (refused at publish by the spec) is handed only its first measure, as before, so its other measures are not read as groups. +- **Unchanged:** a chart with one dimension and one measure draws exactly as before on all four types, a comparison overlay (`compareTo`) is never read as a group, and a chart with no rows is the empty chart it was. A stored dashboard widget of these types that declares several measures (refused at publish by the spec) is handed only its first measure, as before, so its other measures are not read as groups.