diff --git a/.changeset/degenerate-magnitude-charts.md b/.changeset/degenerate-magnitude-charts.md new file mode 100644 index 000000000..77c597f16 --- /dev/null +++ b/.changeset/degenerate-magnitude-charts.md @@ -0,0 +1,20 @@ +--- +'@object-ui/plugin-charts': patch +--- + +Pie, donut, funnel and treemap now say when rows carry no magnitude they can draw. + +These four families size a mark BY its measure, so a row whose value is zero, +negative, `null` or unparseable stays in the data and is given no area. Measured +in Chromium across 74 tiles: an all-zero pie put ZERO non-white pixels on the +page while its DOM carried 31 descendants and a real `svg`; a treemap handed +`40 / null`, `40 / 0` or `40 / -25 / -12` rendered one full-bleed leaf that was +byte-identical to a genuinely one-row treemap; and a funnel handed `40` beside a +`null` drew no segments at all and labelled the tile with the row that had no +value. + +When no row can be sized, these charts now render the file's refusal shell +(`no-positive-magnitude`) instead of a blank tile. When only some rows can be +sized, the chart still draws and carries a note counting the ones it could not. +All-positive charts, charts handed no rows at all, bar charts, and both sankey +answers are unchanged. diff --git a/packages/plugin-charts/src/AdvancedChartImpl.degenerateMagnitude.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.degenerateMagnitude.test.tsx new file mode 100644 index 000000000..1dba3ed37 --- /dev/null +++ b/packages/plugin-charts/src/AdvancedChartImpl.degenerateMagnitude.test.tsx @@ -0,0 +1,281 @@ +/** + * 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. + * + * Pie / donut / funnel / treemap — rows that are NEVER filtered and are given + * no area anyway (objectui#7147). The third mechanism on this surface, and the + * one neither landed answer reaches. + * + * ## Three mechanisms, one reader-facing symptom + * + * - objectui#7140 / objectui#7146 — an early return emitting a bare `div` + * - objectui#7148 — a silent row DROP before the plot + * - objectui#7147 — DEGENERATE GEOMETRY, pinned here + * + * objectui#7148's footnote counts dropped rows (`data.length - rows.length`). + * Against these four families that count is exactly ZERO — the sankey arm holds + * the only row-dropping filter in the file — so hoisting it here would have + * rendered nothing at all while looking like coverage. The rows stay in `data` + * and the LAYOUT gives them no area. Zero area is not zero elements, and + * neither is a dropped row. + * + * ## The measurement that decided fix-over-decline, per family + * + * 74 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main` + * 40c4711d6, each screenshotted, MD5'd and pixel-diffed against a literally + * empty `div` of the same box. `measured-and-declined` was on the table for + * every family, as the card says, and survived for none of the four: + * + * - pie / donut all-zero, all-null, all-negative: ZERO non-white pixels out + * of 124,800 — byte-identical to the empty div, with 31 descendants and a + * real `svg` in the DOM. + * - pie / donut `40` beside a `null`: a FULL circle in the first category's + * colour, 99.35% pixel-identical to a legitimately one-row dataset (the + * 0.654% residue is the `paddingAngle` hairline, not information). + * - funnel `40` beside a `null`: 178 ink pixels — ZERO segments, ONE label, + * and the label reads "Beta", the row with NO value. The row carrying 40 + * drew nothing at all. + * - funnel all-negative: a confident two-band funnel whose mark area + * (220,320) EXCEEDS the all-positive control's (111,881). + * - treemap `40 / null`, `40 / 0` and `40 / -25 / -12`: all three + * BYTE-IDENTICAL (diff 0.000%) to a genuinely one-row treemap. Four + * datasets, one image. + * - treemap all-zero: ONE full-bleed leaf labelled with the LAST category. + * + * The controls are what make those zeros mean anything: an all-zero BAR drew + * 5,128 ink pixels of axes and ticks on the same instrument — which is why bar + * is deliberately untouched and pinned that way below — a two-row pie differed + * from a one-row pie by 9.683% of its pixels, and a two-row treemap from a + * one-row treemap by 38.301%. + * + * ## What this file pins, and why the passing cases are the discriminating half + * + * The refusal and the note are only half the pin. The other half is everything + * that must NOT have moved: the all-positive control of every family keeps its + * exact DOM (no wrapper element), the no-rows case stays byte-for-byte as it + * was (that is the empty-RESULT question, objectui#7130, answered upstream in + * `ObjectChart`), bar keeps its axes, and BOTH landed sankey answers keep + * firing on their own datasets under their own codes. Measured across the same + * 74 tiles: every sankey tile and every bar tile hashed IDENTICALLY before and + * after this change. + */ +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; + +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +import AdvancedChartImpl from './AdvancedChartImpl'; + +afterEach(cleanup); + +const SERIES = [{ dataKey: 'amount', label: 'Amount' }]; + +type Row = Record; + +const renderChart = (chartType: string, data: Row[]) => + render( + , + ); + +const refusalOf = (c: HTMLElement) => c.querySelector('[data-chart-error="no-positive-magnitude"]'); +const noteOf = (c: HTMLElement) => c.querySelector('[data-chart-note="unsized-rows"]'); +const plotOf = (c: HTMLElement) => c.querySelector('[data-slot="chart"]'); + +/** The four families whose layout sizes a mark BY its measure. */ +const MAGNITUDE_FAMILIES = ['pie', 'donut', 'funnel', 'treemap'] as const; + +/** + * Every shape that reaches the layout carrying no positive magnitude. + * + * The copy deliberately names NONE of them, for the reason `no-positive-flow`'s + * docstring gives: a message saying "negative" is false of the `null` row, one + * saying "zero" is false of the unparseable string, and one naming a missing key + * is false of all four others. The predicate is true of every one of them, so + * the predicate is what the sentence names. + */ +const UNSIZABLE: Array<[string, unknown]> = [ + ['a genuine zero', 0], + ['a negative', -25], + ['null', null], + ['undefined', undefined], + ['an unparseable string', 'n/a'], + ['Infinity, which `Number(x) || 0` would let through', 'Infinity'], +]; + +describe('objectui#7147 — no row can be sized: the chart refuses instead of drawing nothing', () => { + for (const family of MAGNITUDE_FAMILIES) { + for (const [label, value] of UNSIZABLE) { + it(`${family}: every row carrying ${label} gets a refusal, not a blank tile`, () => { + const { container } = renderChart(family, [ + { stage: 'Alpha', amount: value }, + { stage: 'Beta', amount: value }, + ]); + const refusal = refusalOf(container); + expect(refusal).not.toBeNull(); + // The message names the KEY and the exact test it failed — the same + // diagnostic pair `no-positive-flow` carries, which is why neither + // needs a console warning. + expect(refusal!.textContent).toContain('amount'); + expect(refusal!.textContent).toContain('above zero'); + // A refusal REPLACES the plot; it does not sit beside one. + expect(plotOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + }); + } + } + + it('a missing measure key on every row refuses too', () => { + const { container } = renderChart('pie', [{ stage: 'Alpha' }, { stage: 'Beta' }]); + expect(refusalOf(container)).not.toBeNull(); + }); + + it('the refusal uses its OWN code, never the sankey arm\'s', () => { + const { container } = renderChart('funnel', [ + { stage: 'Alpha', amount: 0 }, + { stage: 'Beta', amount: 0 }, + ]); + expect(refusalOf(container)).not.toBeNull(); + expect(container.querySelector('[data-chart-error="no-positive-flow"]')).toBeNull(); + }); +}); + +describe('objectui#7147 — SOME rows can be sized: the chart still draws, and says how many it could not', () => { + for (const family of MAGNITUDE_FAMILIES) { + for (const [label, value] of UNSIZABLE) { + it(`${family}: one good row beside ${label} still DRAWS, with a note`, () => { + const { container } = renderChart(family, [ + { stage: 'Alpha', amount: 40 }, + { stage: 'Beta', amount: value }, + ]); + // The plot is not blanked — objectui#7146 pins the analogous "one + // positive row among zeros still draws" for the sankey arm, and the + // same must hold here or a fix becomes a regression. + expect(plotOf(container)).not.toBeNull(); + expect(refusalOf(container)).toBeNull(); + const note = noteOf(container); + expect(note).not.toBeNull(); + expect(note!.getAttribute('role')).toBe('note'); + expect(note!.textContent).toContain('1 of 2 rows has'); + expect(note!.textContent).toContain('amount'); + }); + } + } + + it('the COUNT is the half a reader cannot recover from the picture', () => { + const { container } = renderChart('treemap', [ + { stage: 'Alpha', amount: 40 }, + { stage: 'Beta', amount: -25 }, + { stage: 'Gamma', amount: -12 }, + ]); + // Measured: this dataset rendered ONE full-bleed leaf, byte-identical to a + // genuinely one-row treemap. "Some rows" would leave those two images + // identical in meaning; "2 of 3" is the bit that was missing. + expect(noteOf(container)!.textContent).toContain('2 of 3 rows have'); + }); + + it('the note uses its OWN attribute, never objectui#7148\'s', () => { + const { container } = renderChart('pie', [ + { stage: 'Alpha', amount: 40 }, + { stage: 'Beta', amount: 0 }, + ]); + expect(noteOf(container)).not.toBeNull(); + expect(container.querySelector('[data-chart-note="omitted-rows"]')).toBeNull(); + }); +}); + +describe('objectui#7147 — the cases that must NOT have moved', () => { + for (const family of MAGNITUDE_FAMILIES) { + it(`${family}: an all-positive chart gains no note, no refusal and NO WRAPPER`, () => { + const { container } = renderChart(family, [ + { stage: 'Alpha', amount: 40 }, + { stage: 'Beta', amount: 25 }, + ]); + expect(refusalOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + // `ChartFootnote` with a null note returns its children untouched, so the + // plot stays the FIRST element — no existing caller gains a wrapper. + expect(container.firstElementChild?.getAttribute('data-slot')).toBe('chart'); + }); + + it(`${family}: handed NO rows at all, nothing is said`, () => { + // The empty-RESULT question (objectui#7130) is answered upstream in + // `ObjectChart`, where the query outcome is actually known. "No row's + // measure is above zero" would be a sentence about rows that do not + // exist. + const { container } = renderChart(family, []); + expect(refusalOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + }); + } + + it('bar is untouched: an all-zero bar chart still draws its axes and says nothing', () => { + // Deliberate, and measured: an all-zero bar drew 5,128 ink pixels of axes + // and ticks against a blank tile's 0. Its reader can already tell a zero + // dataset from a broken widget, so bar is outside this card. + const { container } = renderChart('bar', [ + { stage: 'Alpha', amount: 0 }, + { stage: 'Beta', amount: 0 }, + ]); + expect(refusalOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + expect(plotOf(container)).not.toBeNull(); + }); +}); + +describe('objectui#7147 — the seam with the two landed sankey answers, pinned from both sides', () => { + it('an all-zero SANKEY keeps objectui#7146\'s refusal and never gets this one', () => { + const { container } = renderChart('sankey', [ + { stage: 'Alpha', amount: 0 }, + { stage: 'Beta', amount: 0 }, + ]); + expect(container.querySelector('[data-chart-error="no-positive-flow"]')).not.toBeNull(); + expect(refusalOf(container)).toBeNull(); + }); + + it('a thinned SANKEY keeps objectui#7148\'s footnote and never gets this note', () => { + const { container } = renderChart('sankey', [ + { stage: 'Alpha', amount: 40 }, + { stage: 'Beta', amount: -25 }, + ]); + expect(container.querySelector('[data-chart-note="omitted-rows"]')).not.toBeNull(); + expect(noteOf(container)).toBeNull(); + }); + + it('the three codes are mutually exclusive on every dataset in the sweep', () => { + const datasets: Row[][] = [ + [{ stage: 'Alpha', amount: 0 }, { stage: 'Beta', amount: 0 }], + [{ stage: 'Alpha', amount: 40 }, { stage: 'Beta', amount: null }], + [{ stage: 'Alpha', amount: 40 }, { stage: 'Beta', amount: 25 }], + [], + ]; + for (const family of [...MAGNITUDE_FAMILIES, 'sankey', 'bar']) { + for (const data of datasets) { + const { container } = renderChart(family, data); + const codes = [ + container.querySelector('[data-chart-error="no-positive-flow"]'), + container.querySelector('[data-chart-error="no-positive-magnitude"]'), + container.querySelector('[data-chart-note="omitted-rows"]'), + container.querySelector('[data-chart-note="unsized-rows"]'), + ].filter(Boolean).length; + expect(codes).toBeLessThanOrEqual(1); + cleanup(); + } + } + }); +}); diff --git a/packages/plugin-charts/src/AdvancedChartImpl.tsx b/packages/plugin-charts/src/AdvancedChartImpl.tsx index 5e2486b8f..433eb8950 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -407,6 +407,146 @@ function ChartFootnote({ note, children }: { note?: React.ReactNode; children: R ); } +/** + * How many rows a MAGNITUDE chart can actually give area to (objectui#7147). + * + * ## The third mechanism, and why neither landed answer reaches it + * + * Three distinct mechanisms on this surface produce ONE reader-facing symptom — + * a tile that says nothing, or says something false, about rows it was handed: + * + * - an early return that emits a bare `div` — objectui#7140 / objectui#7146 + * - a silent row DROP before the plot — objectui#7148 + * - DEGENERATE GEOMETRY, which is this one — objectui#7147 + * + * The rows here are never filtered. `data` reaches ``, `` and + * `` whole — the sankey arm's is the only row-dropping filter in this + * file — and what happens instead is that a row whose measure is not above zero + * is given no area. objectui#7148's count (`data.length - rows.length`) is + * therefore exactly ZERO against these three families, so hoisting that + * footnote here would render nothing at all while looking like coverage. Zero + * area is not zero elements, and neither is a dropped row. + * + * ## The measurement that decided fix-over-decline, per family + * + * 56 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main` + * 40c4711d6 — six families x nine datasets — each tile screenshotted, MD5'd, + * and pixel-diffed against a literally empty `div` of the same 520x240 box. + * `measured-and-declined` was genuinely on the table for all three families, + * as the card says, and survived for none of them: + * + * - pie / donut, all-zero, all-null, all-negative: ZERO non-white pixels out + * of 124,800. Not blank-LOOKING — byte-identical to the empty div, while + * the DOM carried 31 descendants and a real `svg`. + * - pie / donut, `40` beside a `null`: a FULL circle in the first category's + * colour, 99.35% pixel-identical to a legitimately one-row dataset (diff + * 0.654%, and that residue is the `paddingAngle` hairline, not + * information). The picture asserts "Alpha is 100%" of a dataset in which + * Beta was never measured at all. + * - funnel, `40` beside a `null`: 178 ink pixels — ZERO segments and ONE + * label, and the label is "Beta", the row with NO value. The row carrying + * 40 draws nothing whatsoever. + * - funnel, all-negative: a large, healthy-looking two-band funnel whose mark + * area (220,320) EXCEEDS the all-positive control's (111,881). + * - treemap, `40` beside a `null`, `40` beside a `0`, and mixed-sign + * `40 / -25 / -12`: all three BYTE-IDENTICAL (diff 0.000%) to a genuinely + * one-row treemap — one full-bleed leaf labelled "Alpha". Four datasets, + * one image. + * - treemap, all-zero: one full-bleed leaf labelled "Beta" — the LAST + * category — asserting that one of two equal-zero categories is the entire + * composition. + * + * The controls are what make those zeros readable. On the same instrument an + * all-zero BAR drew 5,128 ink pixels of axes and ticks — which is why bar is + * deliberately NOT touched here: its reader can already tell. A two-row pie + * differed from a one-row pie by 9.683% of its pixels and a two-row treemap + * from a one-row treemap by 38.301%, so the instrument separates these datasets + * easily whenever the renderer does. + * + * ## Why the predicate is `> 0`, and why the copy names it + * + * The reason `no-positive-flow`'s docstring gives. Five shapes reach here — a + * genuine zero, a negative, `null`, an unparseable string, and a missing key — + * and naming any ONE of them is a sentence that is false for the other four. + * A strictly positive, finite measure is the single test all three families' + * layouts effectively apply, so the copy names THAT. + * + * `Number.isFinite(v) && v > 0` rather than the sankey arm's `Number(...) || 0` + * idiom: this predicate must also reject `Infinity`, which has no finite area + * to occupy anywhere and which `|| 0` lets straight through. + */ +function countSizableRows(rows: unknown[], dataKey: string): { sizable: number; total: number } { + let sizable = 0; + for (const row of rows) { + const v = Number((row as Record | null | undefined)?.[dataKey]); + if (Number.isFinite(v) && v > 0) sizable += 1; + } + return { sizable, total: rows.length }; +} + +/** + * The refusal a magnitude chart renders when NO row can be sized. + * + * The same shell and the same shape of sentence as `no-positive-flow`, and + * deliberately a DIFFERENT code: that one is the sankey arm's and answers rows + * being DROPPED, this one answers geometry that collapses with every row still + * present. Sharing a code would make the two indistinguishable to the pins that + * exist to keep them apart. + * + * Callers gate it on `total > 0`, for the reason objectui#7146 gives: handed NO + * rows, "no row's measure is above zero" is a sentence about rows that do not + * 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. + */ +function MagnitudeRefusal({ dataKey, className }: { dataKey: string; className?: string }) { + return ( + + This chart has nothing to size: no row's{' '} + {dataKey} is above zero. + + ); +} + +/** + * The note a magnitude chart carries when SOME rows can be sized and some cannot. + * + * Returns `null` when every row is sizable, and that is the gate which keeps + * healthy charts byte-identical: `ChartFootnote` with no note renders its + * children untouched, so no existing caller gains a wrapper element. + * + * ## Why it does NOT say "showing N of M rows" + * + * objectui#7148's sankey note can say that, because there the missing rows are + * genuinely absent from the plot. Here they are not. A mixed-sign pie PAINTS a + * sector for every row — measured: `40 / -25 / -12` drew 3 sectors — it just + * paints them at a scale that means nothing, and a funnel handed the same rows + * drew 3 trapezoids. "Showing 1 of 3" would be a false statement about what is + * on the screen. What IS true of every one of them is that the chart sizes by + * value and these rows carry no value it can size, so that is what the copy + * says. + * + * `rows` is an unconditional plural because the note cannot render with fewer + * than two: reaching it at all means at least one row was sized (otherwise the + * refusal returned first) and at least one was not. + * + * No console warning, matching the two sankey answers and unlike the two guards + * at the bottom of this file: those carry a diagnostic PAIR that does not fit on + * screen, whereas this sentence already names the key, the test it failed, and + * how many rows failed it. + */ +function unsizedRowsNote(sizable: number, total: number, dataKey: string): React.ReactNode { + const unsized = total - sizable; + if (unsized <= 0) return null; + return ( +

+ {unsized} of {total} rows {unsized === 1 ? 'has' : 'have'} no{' '} + {dataKey} above zero — this chart sizes by value, so{' '} + {unsized === 1 ? 'that row is' : 'those rows are'} not drawn to scale. +

+ ); +} + /** * AdvancedChartImpl - The heavy implementation that imports Recharts with full features * This component is lazy-loaded to avoid including Recharts in the initial bundle @@ -937,6 +1077,16 @@ function AdvancedChartImplInner({ if (chartType === 'pie' || chartType === 'donut') { const innerRadius = chartType === 'donut' ? '52%' : 0; const palette = getPalette(); + // objectui#7147 — see `countSizableRows`. A slice's angle is its share of + // the positive total, so a row that is zero, negative, null or unparseable + // is drawn as nothing at all while staying in `data`. Measured: all-zero, + // all-null and all-negative pies each put ZERO non-white pixels on a + // 520x240 tile, byte-identical to an empty div. + const pieDataKey = series[0]?.dataKey || 'value'; + const pieSizable = countSizableRows(data, pieDataKey); + if (pieSizable.total > 0 && pieSizable.sizable === 0) { + return ; + } // Augment the chart config with one entry per category value so that // `ChartLegendContent` (which resolves item labels via `config[key]`) // can render the slice labels next to the color swatches. Without @@ -954,13 +1104,13 @@ function AdvancedChartImplInner({ }; } }); - return ( + const pieChart = ( } /> ); + // A pie that drew SOME of its rows says how many it could not size. With + // every row sizable the note is `null` and `ChartFootnote` returns the + // container untouched, so healthy pies keep their exact DOM. + return ( + + {pieChart} + + ); } // Funnel chart — uses recharts FunnelChart (single series only) if (chartType === 'funnel') { const dataKey = series[0]?.dataKey || 'value'; const palette = getPalette(); + // objectui#7147 — see `countSizableRows`. Recharts derives each segment's + // upper and lower width from ITS value and the NEXT one, so a single + // unsizable row does not merely omit itself: measured, `40` beside a `null` + // drew ZERO segments and one label reading "Beta" — the row with no value — + // while the row carrying 40 drew nothing at all. All-negative is the mirror + // image: a confident two-band funnel with MORE mark area than the + // all-positive control. + const funnelSizable = countSizableRows(data, dataKey); + if (funnelSizable.total > 0 && funnelSizable.sizable === 0) { + return ; + } const handleFunnelClick = onChartClick ? (entry: any) => { if (!entry) return; @@ -1031,7 +1200,7 @@ function AdvancedChartImplInner({ const bv = Number(b?.[dataKey] ?? 0); return bv - av; }); - return ( + const funnelChart = ( } /> @@ -1050,6 +1219,11 @@ function AdvancedChartImplInner({ ); + return ( + + {funnelChart} + + ); } // Treemap — composition by relative size. Recharts is itself the @@ -1058,18 +1232,33 @@ function AdvancedChartImplInner({ if (chartType === 'treemap') { const dataKey = series[0]?.dataKey || 'value'; const palette = getPalette(); + // objectui#7147 — see `countSizableRows`. A treemap's leaf area IS its + // value, so an unsizable row collapses to nothing and its neighbours expand + // to fill the box. Measured: `40 / null`, `40 / 0` and `40 / -25 / -12` all + // rendered ONE full-bleed leaf labelled "Alpha", byte-identical to each + // other AND to a genuinely one-row treemap. All-zero rows paint one + // full-bleed leaf labelled with the LAST category. + const treemapSizable = countSizableRows(data, dataKey); + if (treemapSizable.total > 0 && treemapSizable.sizable === 0) { + return ; + } const tmData = data.map((row, idx) => ({ name: String(row?.[xAxisKey] ?? ''), size: Number(row?.[dataKey]) || 0, fill: resolveColor(palette[idx % palette.length]), })); - return ( + const treemapChart = ( } {...treemapClickProps}> ); + return ( + + {treemapChart} + + ); } // Sankey — flow from a single root node to each category, weighted by value.