diff --git a/.changeset/scatter-unplaceable-points.md b/.changeset/scatter-unplaceable-points.md new file mode 100644 index 000000000..aa619cb44 --- /dev/null +++ b/.changeset/scatter-unplaceable-points.md @@ -0,0 +1,21 @@ +--- +'@object-ui/plugin-charts': patch +--- + +Scatter now says when it cannot place a row, instead of drawing an empty axis. + +Scatter is the only two-measure positional chart in the renderer: `xAxisKey` feeds +a numeric X axis and `series[0]` a numeric Y axis, so a point exists only when +both are numbers. Measured in real Chromium, rows it could not place produced a +tile byte-identical to a scatter handed no rows at all, and six different +authoring failures shared one image. A chart with one placeable row among three +was 99.75% pixel-identical to a genuinely one-row scatter. + +Handed rows it cannot place any of, a scatter now renders the file's refusal +shell under `data-chart-error="no-plottable-points"`, naming both keys. When some +rows place and some do not it draws as before with a `data-chart-note="unplotted-points"` +footnote carrying the count. Charts whose rows all place are byte-identical to +before, and no wrapper element is added to them. + +The predicate is positional, not magnitude-based: zero and negative coordinates +are ordinary scatter data and keep drawing. diff --git a/packages/plugin-charts/src/AdvancedChartImpl.degenerateMagnitude.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.degenerateMagnitude.test.tsx index 1dba3ed37..ddcb81b45 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.degenerateMagnitude.test.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.degenerateMagnitude.test.tsx @@ -257,25 +257,54 @@ describe('objectui#7147 — the seam with the two landed sankey answers, pinned expect(noteOf(container)).toBeNull(); }); - it('the three codes are mutually exclusive on every dataset in the sweep', () => { + it('the codes are mutually exclusive on every dataset in the sweep', () => { + // Extended by objectui#7171 rather than duplicated beside: scatter and its + // two codes join the SAME exclusivity assertion, so a sixth answer cannot + // be added later without this test having an opinion about it. 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) { + // Scatter reads TWO measures, so the sweep's single-measure rows are also + // the dataset that leaves it unplaceable — which is exactly the pair this + // assertion has to keep apart. + const scatterDatasets: Row[][] = [ + ...datasets, + [{ stage: 1, amount: 40 }, { stage: 2, amount: 25 }], + [{ stage: 1, amount: 40 }, { stage: 2, amount: null }], + ]; + for (const family of [...MAGNITUDE_FAMILIES, 'sankey', 'bar', 'scatter']) { + for (const data of family === 'scatter' ? scatterDatasets : 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-error="no-plottable-points"]'), container.querySelector('[data-chart-note="omitted-rows"]'), container.querySelector('[data-chart-note="unsized-rows"]'), + container.querySelector('[data-chart-note="unplotted-points"]'), ].filter(Boolean).length; - expect(codes).toBeLessThanOrEqual(1); + expect(codes, `${family} / ${JSON.stringify(data)}`).toBeLessThanOrEqual(1); cleanup(); } } }); + + it('scatter never receives a MAGNITUDE code, and a magnitude family never receives scatter\'s', () => { + // objectui#7171. Same fence as the sankey pair above: pie sizes by + // magnitude and scatter plots by position, so an all-negative dataset is a + // refusal for one and perfectly ordinary data for the other. + const negatives: Row[] = [{ stage: -10, amount: -40 }, { stage: -20, amount: -25 }]; + const { container: scatter } = renderChart('scatter', negatives); + expect(scatter.querySelector('[data-chart-error="no-plottable-points"]')).toBeNull(); + expect(refusalOf(scatter)).toBeNull(); + expect(noteOf(scatter)).toBeNull(); + cleanup(); + + const { container: pie } = renderChart('pie', negatives); + expect(refusalOf(pie)).not.toBeNull(); + expect(pie.querySelector('[data-chart-error="no-plottable-points"]')).toBeNull(); + }); }); diff --git a/packages/plugin-charts/src/AdvancedChartImpl.degeneratePosition.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.degeneratePosition.test.tsx new file mode 100644 index 000000000..38a22137b --- /dev/null +++ b/packages/plugin-charts/src/AdvancedChartImpl.degeneratePosition.test.tsx @@ -0,0 +1,405 @@ +/** + * 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. + * + * Scatter — rows that cannot be PLACED (objectui#7171). The fourth mechanism on + * this surface, and the first positional one. + * + * ## Why this card exists at all: a control that returns zero is no control + * + * objectui#7147 swept eight chart families for degenerate magnitude and reached + * a verdict for seven. Scatter's every tile drew ZERO marks — its all-positive + * CONTROL included — because scatter takes TWO measures and that sweep's + * fixture supplied one. With the control dead, none of scatter's zeros carried + * information: they were indistinguishable from "the fixture never bound a + * scatter at all". Reporting scatter as clean on that evidence would have been + * a false green across nine datasets, so it was reported NOT MEASURED instead — + * which is neither red nor green. + * + * The first test below is therefore the load-bearing one: it asserts the + * control DRAWS. If it ever goes red, every other assertion in this file is + * void rather than passing, and that is the point of stating it as an assertion + * instead of an eyeballed observation. + * + * ## What a correct measurement found + * + * 33 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main` + * 899730e0a, one page load each, screenshotted, MD5'd and pixel-diffed against + * a literally empty `div` of the same 520x360 box — above the chart's own + * `CHART_MIN_HEIGHT` floor of 280, below which no footnote is visible at all. + * The two-measure control drew 3 of 3 marks (245px of mark area). Then: + * + * - rows whose x AND y are both unplaceable rendered BYTE-IDENTICALLY (diff + * 0.000%) to a scatter handed NO ROWS AT ALL — the empty-result picture, + * for a query that returned rows; + * - `null` x, absent x, `'n/a'` x, `'Infinity'` x, boolean x and + * objectui#7147's own category-column fixture were SIX datasets sharing ONE + * image (`51957063d9c2`): a confident y scale, an x axis with no scale at + * all, and no marks; + * - one plottable row among three was 99.75% pixel-identical to a genuinely + * one-row scatter (diff 0.250%) — the same collision that decided pie. + * + * After the change: 15 tiles moved, 18 are byte-identical, and the two + * collisions above are broken (0.000% to 2.651%, and 0.250% to 6.368%). + * + * ## Measured and DECLINED, so nothing here pins a guard for it + * + * ZERO VARIANCE — three rows at one coordinate. A2.3 predicted axis domain + * collapse and there is none: Recharts pads the domain exactly as for one row + * (x ticks `0,3,6,9,12` in both), draws all three symbols, and the tile is + * 99.98% identical to a one-row scatter because three coincident points ARE one + * dot. That is overplotting — a property of the form, and a TRUE picture. + */ +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); + +type Row = Record; + +const SERIES = [{ dataKey: 'ym', label: 'Y' }]; + +/** Both axes bound to a numeric measure — the fixture objectui#7147 lacked. */ +const renderScatter = (data: Row[], props: Record = {}) => + render( + , + ); + +const refusalOf = (c: HTMLElement) => c.querySelector('[data-chart-error="no-plottable-points"]'); +const noteOf = (c: HTMLElement) => c.querySelector('[data-chart-note="unplotted-points"]'); +const plotOf = (c: HTMLElement) => c.querySelector('[data-slot="chart"]'); +/** Recharts paints one `path.recharts-symbols` per PLACED point. */ +const marksOf = (c: HTMLElement) => c.querySelectorAll('path.recharts-symbols').length; + +/** Three rows, both coordinates finite on every one of them. */ +const CONTROL: Row[] = [{ xm: 10, ym: 40 }, { xm: 20, ym: 25 }, { xm: 35, ym: 60 }]; + +/** + * Every shape that reaches a numeric axis carrying nothing it can place. + * + * `Number()` alone gets the first two WRONG in the permissive direction — + * `Number(null) === 0` and `Number(true) === 1` are both finite — and Recharts + * plots neither. Measured, not assumed: 0 of 2 marks each. + */ +const UNPLACEABLE: Array<[string, unknown]> = [ + ['null, which `Number()` would call a finite 0', null], + ['a missing key', undefined], + ['an unparseable string', 'n/a'], + ['Infinity', 'Infinity'], +]; + +describe('objectui#7171 — THE CONTROL. Every other assertion in this file depends on it', () => { + it('a TWO-measure scatter actually draws marks', () => { + // objectui#7147's single-measure fixture drew ZERO here, which is why + // scatter came out of that sweep NOT MEASURED. A control that returns zero + // is no control, so this is asserted rather than observed: if it goes red, + // every zero below is void, not green. + const { container } = renderScatter(CONTROL); + expect(marksOf(container), 'the all-positive control MUST draw').toBeGreaterThan(0); + expect(marksOf(container)).toBe(3); + }); + + it('objectui#7147\'s own fixture is reproduced, and it is the fixture that was broken', () => { + // A category column on a `type="number"` axis. Same renderer, same head — + // the difference between this and the control is the FIXTURE, which is the + // whole claim of the card. + const { container } = render( + , + ); + expect(marksOf(container)).toBe(0); + }); +}); + +describe('objectui#7171 — no row can be placed: the chart says so instead of drawing an empty axis', () => { + for (const [label, value] of UNPLACEABLE) { + it(`x is ${label} on every row: a refusal, not a frame with no marks`, () => { + const { container } = renderScatter([{ xm: value, ym: 40 }, { xm: value, ym: 25 }]); + const refusal = refusalOf(container); + expect(refusal).not.toBeNull(); + // A point needs BOTH coordinates, so the message names both keys. + expect(refusal!.textContent).toContain('xm'); + expect(refusal!.textContent).toContain('ym'); + expect(plotOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + }); + + it(`y is ${label} on every row: the same refusal — a point needs BOTH`, () => { + const { container } = renderScatter([{ xm: 10, ym: value }, { xm: 20, ym: value }]); + expect(refusalOf(container)).not.toBeNull(); + }); + + it(`BOTH coordinates are ${label}: refused, not shown the empty-result picture`, () => { + // Measured at 899730e0a: this rendered BYTE-IDENTICALLY (0.000%) to a + // scatter handed no rows at all. That collision is the defect. + const { container } = renderScatter([{ xm: value, ym: value }, { xm: value, ym: value }]); + expect(refusalOf(container)).not.toBeNull(); + }); + } + + it('rows carrying neither key at all are refused', () => { + const { container } = renderScatter([{ other: 1 }, { other: 2 }]); + expect(refusalOf(container)).not.toBeNull(); + }); + + it('the refusal names the keys it was actually given, not a hardcoded pair', () => { + const { container } = renderScatter( + [{ lat: null, lng: null }], + { xAxisKey: 'lat', series: [{ dataKey: 'lng' }] }, + ); + expect(refusalOf(container)!.textContent).toContain('lat'); + expect(refusalOf(container)!.textContent).toContain('lng'); + }); + + it('with no series declared the y key falls back to `value`, and the refusal says so', () => { + // The arm binds `series[0]?.dataKey || 'value'` ONCE and hands it to both + // the predicate and the YAxis, so the two cannot drift. + const { container } = renderScatter( + [{ xm: 10, value: null }, { xm: 20, value: null }], + { series: [] }, + ); + expect(refusalOf(container)!.textContent).toContain('value'); + }); +}); + +describe('objectui#7171 — a POSITIONAL chart legitimately plots zero and negative numbers', () => { + // The fence this card was opened with. Pie, funnel and treemap size BY + // magnitude, so objectui#7147's `Number.isFinite(v) && v > 0` is right there. + // Scatter plots position: a chart of temperatures or profit deltas is + // SUPPOSED to show these. Reusing that predicate would refuse correct charts. + const LEGITIMATE: Array<[string, Row[], number]> = [ + ['all-negative coordinates', [{ xm: -10, ym: -40 }, { xm: -20, ym: -25 }, { xm: -35, ym: -60 }], 3], + ['every coordinate zero', [{ xm: 0, ym: 0 }, { xm: 0, ym: 0 }], 2], + ['mixed signs including a zero', [{ xm: 10, ym: 40 }, { xm: -25, ym: -12 }, { xm: 0, ym: 5 }], 3], + // `''` is the mirror-image trap: it LOOKS like a null, `Number('') === 0`, + // and Recharts DOES plot it at zero (measured, 2 of 2 marks). Rejecting it + // would blank a chart that draws. + ['an empty string, which Recharts places at zero', [{ xm: '', ym: 40 }, { xm: '', ym: 25 }], 2], + ['numeric strings', [{ xm: '10', ym: 40 }, { xm: '20', ym: 25 }], 2], + ]; + + for (const [label, data, expected] of LEGITIMATE) { + it(`${label}: draws every mark, and says nothing`, () => { + const { container } = renderScatter(data); + expect(marksOf(container), 'all marks drawn').toBe(expected); + expect(refusalOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + }); + } + + it('zero variance is measured-and-DECLINED: three coincident points are one honest dot', () => { + // A2.3 predicted axis domain collapse. There is none — Recharts pads the + // domain exactly as it does for one row, and the picture is TRUE. + const { container } = renderScatter([{ xm: 10, ym: 40 }, { xm: 10, ym: 40 }, { xm: 10, ym: 40 }]); + expect(marksOf(container)).toBe(3); + expect(refusalOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + }); + + it('a BOOLEAN coordinate is measured-and-DECLINED, because whether it places depends on its neighbours', () => { + // The trap this card was fenced against, hit in a new place. An all-boolean + // x drew 0 of 2 marks in the browser sweep and looked exactly like a + // sibling of `null` — and pinning it as unplaceable turned RED here, which + // is how the mixed reading was found: Recharts needs one real number to + // build the scale and then coerces the booleans onto it. + const { container: mixed } = renderScatter([ + { xm: 10, ym: 40 }, + { xm: true, ym: 25 }, + { xm: false, ym: 60 }, + ]); + expect(marksOf(mixed), 'a boolean beside a number DOES place').toBe(3); + // So the predicate must accept it: a footnote reading "2 of 3 rows are not + // drawn" over three visible points is a false sentence about the picture, + // which is worse than the silence this card is about. + expect(noteOf(mixed)).toBeNull(); + expect(refusalOf(mixed)).toBeNull(); + cleanup(); + + // The cost of that choice, stated rather than hidden: with EVERY row + // boolean there is no scale to coerce onto, nothing draws, and this card's + // answer stays silent — exactly as it is today. A narrow hole, not a + // regression, and its real answer belongs upstream. + const { container: allBool } = renderScatter([{ xm: true, ym: 40 }, { xm: false, ym: 25 }]); + expect(marksOf(allBool)).toBe(0); + expect(refusalOf(allBool)).toBeNull(); + }); + + it('a constant x with a varying y still draws every mark', () => { + const { container } = renderScatter([{ xm: 10, ym: 40 }, { xm: 10, ym: 25 }, { xm: 10, ym: 60 }]); + expect(marksOf(container)).toBe(3); + expect(refusalOf(container)).toBeNull(); + }); +}); + +describe('objectui#7171 — SOME rows can be placed: it still draws, and says how many it could not', () => { + for (const [label, value] of UNPLACEABLE) { + it(`one placeable row beside two whose x is ${label}: draws, with a note`, () => { + const { container } = renderScatter([ + { xm: 10, ym: 40 }, + { xm: value, ym: 25 }, + { xm: value, ym: 60 }, + ]); + expect(plotOf(container)).not.toBeNull(); + expect(refusalOf(container)).toBeNull(); + // Measured: exactly ONE symbol for three rows — the missing points really + // are absent from the picture, which is what lets the copy say so. + expect(marksOf(container)).toBe(1); + const note = noteOf(container); + expect(note).not.toBeNull(); + expect(note!.getAttribute('role')).toBe('note'); + expect(note!.textContent).toContain('2 of 3 rows have'); + expect(note!.textContent).toContain('not drawn'); + }); + } + + it('the COUNT is the half a reader cannot recover from the picture', () => { + // Measured at 899730e0a: this tile was 99.75% pixel-identical to a + // genuinely one-row scatter (diff 0.250%). "Some rows" would leave those + // two pictures identical in meaning; "2 of 3" is the bit that was missing. + const { container } = renderScatter([ + { xm: 10, ym: 40 }, + { xm: null, ym: 25 }, + { xm: null, ym: 60 }, + ]); + expect(noteOf(container)!.textContent).toContain('2 of 3 rows have'); + }); + + it('a single unplaceable row reads in the singular', () => { + const { container } = renderScatter([{ xm: 10, ym: 40 }, { xm: 20, ym: null }]); + expect(noteOf(container)!.textContent).toContain('1 of 2 rows has'); + expect(noteOf(container)!.textContent).toContain('that point is'); + }); + + it('a row unplaceable on y alone counts too — a point needs both', () => { + const { container } = renderScatter([ + { xm: 10, ym: 40 }, + { xm: 20, ym: 'n/a' }, + { xm: 35, ym: 'n/a' }, + ]); + expect(marksOf(container)).toBe(1); + expect(noteOf(container)!.textContent).toContain('2 of 3 rows have'); + }); +}); + +describe('objectui#7171 — the cases that must NOT have moved', () => { + it('an all-placeable scatter gains no note, no refusal and NO WRAPPER', () => { + const { container } = renderScatter(CONTROL); + 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('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. + const { container } = renderScatter([]); + expect(refusalOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + expect(plotOf(container)).not.toBeNull(); + }); + + it('a one-row scatter is untouched', () => { + const { container } = renderScatter([{ xm: 10, ym: 40 }]); + expect(marksOf(container)).toBe(1); + expect(refusalOf(container)).toBeNull(); + expect(noteOf(container)).toBeNull(); + }); +}); + +describe('objectui#7171 — the seam with the three landed answers, pinned from both sides', () => { + it('a scatter that cannot place a row never receives a MAGNITUDE code', () => { + const { container } = renderScatter([{ xm: null, ym: null }, { xm: null, ym: null }]); + expect(refusalOf(container)).not.toBeNull(); + expect(container.querySelector('[data-chart-error="no-positive-magnitude"]')).toBeNull(); + expect(container.querySelector('[data-chart-error="no-positive-flow"]')).toBeNull(); + expect(container.querySelector('[data-chart-note="unsized-rows"]')).toBeNull(); + expect(container.querySelector('[data-chart-note="omitted-rows"]')).toBeNull(); + }); + + it('an all-negative scatter DRAWS where an all-negative pie refuses', () => { + // The two halves of the same fence, in one test: identical data, opposite + // correct answers, because one chart sizes and the other places. + const rows = [{ xm: -10, ym: -40 }, { xm: -20, ym: -25 }]; + const { container: sc } = renderScatter(rows); + expect(marksOf(sc)).toBe(2); + expect(refusalOf(sc)).toBeNull(); + cleanup(); + + const { container: pie } = render( + , + ); + expect(pie.querySelector('[data-chart-error="no-positive-magnitude"]')).not.toBeNull(); + }); + + it('a magnitude family never receives THIS code', () => { + for (const family of ['pie', 'donut', 'funnel', 'treemap', 'sankey', 'bar']) { + const { container } = render( + , + ); + expect(refusalOf(container), `${family} must not get no-plottable-points`).toBeNull(); + expect(noteOf(container), `${family} must not get unplotted-points`).toBeNull(); + cleanup(); + } + }); + + it('scatter is still outside the category-axis and series-axis guards', () => { + // Both are pinned elsewhere; re-asserted here because this card adds the + // first refusal scatter has ever had, and the seam must stay three-way. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container } = renderScatter([{ xm: 10, ym: 40 }, { xm: 20, ym: 25 }]); + expect(container.querySelector('[data-chart-error="missing-category-key"]')).toBeNull(); + expect(container.querySelector('[data-chart-error="no-plottable-series"]')).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it('neither response prints a console warning, matching all three landed answers', () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + renderScatter([{ xm: null, ym: null }, { xm: null, ym: null }]); + cleanup(); + renderScatter([{ xm: 10, ym: 40 }, { xm: null, ym: 25 }]); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); +}); diff --git a/packages/plugin-charts/src/AdvancedChartImpl.tsx b/packages/plugin-charts/src/AdvancedChartImpl.tsx index 433eb8950..aceb44fcb 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -547,6 +547,197 @@ function unsizedRowsNote(sizable: number, total: number, dataKey: string): React ); } +/** + * Whether a value can be PLACED on one of scatter's two numeric axes + * (objectui#7171). + * + * ## Why this is NOT `countSizableRows` with a different name + * + * Pie, funnel and treemap size a mark BY its measure, so `> 0` is the whole + * test there. Scatter plots POSITION: a negative or zero coordinate is + * perfectly ordinary data — temperatures, profit deltas, a variance around a + * mean — and mechanically reusing objectui#7147's predicate here would REFUSE + * correct charts, which is worse than the silence this card was opened about. + * Measured on the sweep below rather than argued: an all-negative scatter drew + * 3 of 3 marks, an all-zero scatter drew 2 of 2, and a mixed-sign scatter drew + * 3 of 3 — three datasets `no-positive-magnitude` would have blanked outright. + * + * ## Every clause here was forced by a measurement, not by `Number()` + * + * `Number(v)` alone gets three of these WRONG, in both directions, which is why + * the two rejections below are spelled out and the acceptance is not: + * + * - `null` — `Number(null) === 0`, which is finite, so `Number()` alone calls + * it plottable. Recharts draws NOTHING for it (measured: 0 of 2 marks, and + * the axis renders no scale at all). REJECTED here. + * - `''` — the mirror-image trap. It LOOKS like a null and reads like one, + * and `Number('') === 0`. Recharts DOES plot it, at zero (measured: 2 of 2 + * marks, x ticks `0,1,2,3,4`). So it stays PLOTTABLE: rejecting it would + * blank a chart that draws, which is the failure mode this whole guard + * exists to avoid. + * + * `Number.isFinite` covers the rest as measured: `'10'` plots (2 of 2 marks), + * `'n/a'` and `'Infinity'` and an absent key do not (0 of 2 each). + * + * ## The one shape this predicate deliberately does NOT reject, and why + * + * A BOOLEAN coordinate was rejected in the first draft of this function — the + * browser sweep had measured an all-boolean x drawing 0 of 2 marks, so it + * looked like a sibling of `null`. Pinning it turned that red: a boolean beside + * a genuinely numeric row draws EVERY mark (measured: 3 of 3). Recharts needs + * one real number to build the scale and then coerces the booleans onto it, so + * whether a boolean places depends on the OTHER rows. + * + * Rejecting it is therefore the worse error of the two available. The all- + * boolean tile would gain a correct refusal, but the mixed tile would gain a + * footnote reading "2 of 3 rows ... are not drawn" while all three are on + * screen — a sentence that is simply false about the picture, which is the + * failure this whole family of answers exists to remove. Accepting it leaves + * the all-boolean case exactly as silent as it is today: a narrow hole, not a + * regression, and one whose real answer belongs upstream where a boolean column + * bound to a numeric measure could be refused at authoring time. + */ +function isPlottableCoord(v: unknown): boolean { + if (v === null || v === undefined) return false; + return Number.isFinite(Number(v)); +} + +/** + * How many rows scatter can actually PLACE — a point needs BOTH coordinates + * (objectui#7171). + * + * ## The fourth mechanism on this surface + * + * Four distinct mechanisms now produce one reader-facing symptom, a tile that + * says nothing or says something false about the rows it was handed: + * + * - an early return emitting a bare `div` — objectui#7140 / objectui#7146 + * - a silent row DROP before the plot — objectui#7148 + * - degenerate GEOMETRY, magnitude — objectui#7147 + * - an unplaceable POINT, position — this one + * + * It is genuinely a fourth. objectui#7147's rows are never filtered and are + * painted at a meaningless scale; scatter's unplaceable rows are simply ABSENT + * from the picture — measured, a 3-row dataset with one plottable pair emitted + * exactly ONE `path.recharts-symbols`. That difference is why this note can + * honestly say the points are "not drawn" where objectui#7147's deliberately + * cannot. + * + * ## The measurement that decided fix-over-decline + * + * 33 tiles in real Chromium (`/opt/pw-browsers/chromium`) at `origin/main` + * 899730e0a, one page load each, screenshotted, MD5'd and pixel-diffed against + * a literally empty `div` of the same 520x360 box — a box above the chart's own + * `CHART_MIN_HEIGHT` floor of 280, without which no footnote is visible at all. + * The whole reason this card exists is that objectui#7147's sweep read scatter + * through a SINGLE-measure fixture, so its all-positive control drew zero marks + * and none of its zeros carried information. This fixture binds both axes and + * its control DREW: 3 of 3 marks, 245px of mark area. What it then found: + * + * - rows whose x AND y are both unplaceable render BYTE-IDENTICALLY (diff + * 0.000%) to a scatter handed NO ROWS AT ALL. The reader is shown the + * empty-result picture for a query that returned rows. + * - `null` x, absent x, `'n/a'` x, `'Infinity'` x, boolean x, and + * objectui#7147's own category-column fixture — SIX datasets, ONE image + * (`51957063d9c2`): an axis frame with a confident y scale and no marks. + * Five of those six are answered below; the boolean one deliberately is + * not, for the reason `isPlottableCoord` gives. + * - one plottable row among three is 99.75% pixel-identical to a genuinely + * one-row scatter (diff 0.250%), the same shape of collision that decided + * pie in objectui#7147. Two rows vanish and the picture says "one point". + * + * ## Measured and DECLINED, so it is not guarded here + * + * - ZERO VARIANCE — three rows at the same coordinate. A2.3 expected axis + * domain collapse; there is none. Recharts pads the domain exactly as it + * does for one row (x ticks `0,3,6,9,12` in both), draws 3 symbols, and the + * tile is 99.98% identical to a one-row scatter because three coincident + * points ARE one dot. That is overplotting, a property of the form itself, + * and the picture is TRUE — so there is nothing here to refuse. + * - a CONSTANT x or a constant y with the other varying: both draw every + * mark, with full scales on both axes. + */ +function countPlottablePoints( + rows: unknown[], + xKey: string, + yKey: string, +): { plottable: number; total: number } { + let plottable = 0; + for (const row of rows) { + const r = row as Record | null | undefined; + if (isPlottableCoord(r?.[xKey]) && isPlottableCoord(r?.[yKey])) plottable += 1; + } + return { plottable, total: rows.length }; +} + +/** + * The refusal scatter renders when NO row can be placed. + * + * Its OWN code, for the reason objectui#7147 gives about its own: sharing one + * with the magnitude families would make the two indistinguishable to the pins + * that exist to keep them apart, and they answer different questions — that one + * is about area, this one about position. + * + * It names BOTH keys because a point needs both, and it names the PREDICATE + * rather than a cause: five shapes reach here (a `null`, an absent key, an + * unparseable string, `Infinity`, and a category column on a numeric axis) and + * naming any one of them is a sentence false for the other four. + * + * The caller gates it on `total > 0` — handed no rows, "no row carries a + * number" is a sentence about rows that do not exist. That is the empty-RESULT + * question (objectui#7130), answered upstream in `ObjectChart`. + * + * No console warning, matching all three landed answers in this file. + */ +function PositionRefusal({ + xKey, + yKey, + className, +}: { xKey: string; yKey: string; className?: string }) { + return ( + + This chart has nothing to place: no row carries a number for both{' '} + {xKey} and{' '} + {yKey}. + + ); +} + +/** + * The note scatter carries when SOME rows can be placed and some cannot. + * + * Returns `null` when every row is plottable, and that gate is what keeps + * healthy charts byte-identical: `ChartFootnote` with no note renders its + * children untouched, so no existing caller gains a wrapper element. + * + * ## Why this one CAN say the points are not drawn + * + * objectui#7147's note deliberately does not, because a mixed-sign pie PAINTS a + * sector for every row and only scales them meaninglessly. Scatter is the other + * case and it was measured: a 3-row dataset with one plottable pair emitted + * exactly ONE `path.recharts-symbols`, and the resulting tile was 99.75% + * pixel-identical to a genuinely one-row scatter. The rows really are absent + * from the picture, so saying so is a true statement about what is on screen — + * and the COUNT is the half a reader cannot recover from it. + */ +function unplottedPointsNote( + plottable: number, + total: number, + xKey: string, + yKey: string, +): React.ReactNode { + const unplotted = total - plottable; + if (unplotted <= 0) return null; + return ( +

+ {unplotted} of {total} rows {unplotted === 1 ? 'has' : 'have'} no number for both{' '} + {xKey} and{' '} + {yKey} — this chart plots by position, so{' '} + {unplotted === 1 ? 'that point is' : 'those points are'} not drawn. +

+ ); +} + /** * AdvancedChartImpl - The heavy implementation that imports Recharts with full features * This component is lazy-loaded to avoid including Recharts in the initial bundle @@ -1425,7 +1616,20 @@ function AdvancedChartImplInner({ // Scatter chart if (chartType === 'scatter') { + // objectui#7171 — see `countPlottablePoints`. Scatter is the file's only + // two-measure POSITIONAL family: `xAxisKey` feeds a `type="number"` XAxis + // and `series[0]` a `type="number"` YAxis, so a point exists only if BOTH + // are numbers. The y key is bound once here and handed to both the + // predicate and the axis below, so the two cannot drift apart. + const scatterYKey = series[0]?.dataKey || 'value'; + const points = countPlottablePoints(data, xAxisKey, scatterYKey); + if (points.total > 0 && points.plottable === 0) { + return ; + } return ( + @@ -1439,7 +1643,7 @@ function AdvancedChartImplInner({ /> + ); } diff --git a/packages/plugin-charts/src/AdvancedChartImpl.unprojectedSeriesDimension.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.unprojectedSeriesDimension.test.tsx index d486f91d3..7b72010d8 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.unprojectedSeriesDimension.test.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.unprojectedSeriesDimension.test.tsx @@ -134,12 +134,37 @@ describe('AdvancedChartImpl — an unprojected SECOND dimension refuses (objectu warn.mockRestore(); }); + it('scatter handed a CATEGORY column refuses under objectui#7171, never under this guard', () => { + // The seam, pinned from this side too. Before objectui#7171 this dataset + // drew an axis frame with no marks and said nothing — six such datasets + // shared one image. It is still not a `no-plottable-series` failure: the + // `value` fallback works exactly as this file says it does, and what is + // missing is a plottable X, which is a different sentence. + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const { container } = render( + , + ); + expect(refusal(container), 'not this guard').toBeNull(); + expect( + container.querySelector('[data-chart-error="no-plottable-points"]'), + 'objectui#7171 answers it instead', + ).not.toBeNull(); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + it('stays out of the way of families that draw from a `value` column', () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); // pie / donut / funnel / radar / scatter / treemap / sankey all fall back to // `series[0]?.dataKey || 'value'`, so no series is not no chart there — a // refusal would blank a working one. - for (const chartType of ['pie', 'donut', 'funnel', 'radar', 'scatter']) { + for (const chartType of ['pie', 'donut', 'funnel', 'radar']) { const { container } = render( , + ); + expect(refusal(scatter), 'scatter draws').toBeNull(); + expect(scatter.querySelector('svg'), 'scatter paints').not.toBeNull(); + cleanup(); + expect(warn).not.toHaveBeenCalled(); warn.mockRestore(); });