diff --git a/.changeset/dataset-chartconfig-seam-pin-9203.md b/.changeset/dataset-chartconfig-seam-pin-9203.md new file mode 100644 index 0000000000..3316070923 --- /dev/null +++ b/.changeset/dataset-chartconfig-seam-pin-9203.md @@ -0,0 +1,9 @@ +--- +--- + +Test-only change (objectui#9203): adds a drawn dashboard-seam pin for the five +plot-internal `chartConfig` keys the DATASET face forwards — `colors`, +`categoryColors`, `showDataLabels`, `annotations`, `interaction` — and corrects +the three header comments that cited `plugin-charts`' +`ChartRenderer.dashboardChartConfig.test.tsx` as that face's coverage. No +published behaviour changes; every file touched is a test. diff --git a/packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx b/packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx index 3a36dee8f1..ead34f530f 100644 --- a/packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx +++ b/packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx @@ -16,8 +16,17 @@ * a DOM pin — and the marks below (bars, LabelList, ReferenceLine/Area, Brush) * only exist once Recharts has a measured box. `ResponsiveContainer` reports 0×0 * under the headless DOM and renders no children, and `recharts` resolves inside - * THIS package alone, so the mock that fixes its size — and therefore this half - * of the evidence — has to live here. + * THIS package alone, so a `vi.mock('recharts')` can only be written here. + * + * ⚠️ That premise is true and its old conclusion — "and therefore this half of + * the evidence has to live here" — was not. objectui#9203 re-measured both: + * `require.resolve('recharts')` from `packages/plugin-dashboard` is still + * `MODULE_NOT_FOUND` (with `react` from the same root resolving, as the control + * that the resolver was answering at all), and a module mock turned out not to + * be needed. `ResponsiveContainer` seeds its size from `getBoundingClientRect` + * on its OWN element, so a stub scoped to the `recharts-responsive-container` + * element gives the plot a box inside plugin-dashboard. See "What this file is + * NOT" below for why that distinction decides what this file may be cited for. * * These render `ChartRenderer`, not `AdvancedChartImpl`: `ChartRenderer` is what * the ComponentRegistry resolves `type: 'chart'` to, so it is the component the @@ -25,22 +34,37 @@ * shape `DatasetWidget` emits (derived `chartType`/`xAxisKey`/`series` + * `isAnimationActive: false` + the lowered presentation keys). The seam that * produces it is pinned in plugin-dashboard's - * `DatasetWidget.chartConfig.test.tsx`; together the two close the loop from - * dashboard metadata to drawn pixels. + * `DatasetWidget.chartConfig.test.tsx`. + * + * ⛔ Those two do NOT close the loop from dashboard metadata to drawn pixels, + * and this file's own header used to say they did. "Byte-for-byte the shape + * `DatasetWidget` emits" is a resemblance an author maintains by hand, not a + * composition — nothing below ever calls `DatasetWidget`. See the next block. + * + * ⭐ What this file is NOT (objectui#4044, objectui#9203) — ⛔ it is not + * coverage for ANY dashboard face, and that has now been measured on both. + * + * Three dashboard surfaces lower the same keys through the same + * `chartConfigPresentation` whitelist onto a node of the same shape: the two + * INLINE relays (`DashboardRenderer` and `DashboardGridLayout`, for widgets + * bound to inline rows or to a `provider: 'object'` aggregate) and the DATASET + * face (`DatasetWidget`, for an ADR-0021 dataset). It is tempting to read the + * assertions below as covering them. They do not, and both readings are + * ablations rather than arguments — with the forwarding deleted, the dashboard + * files reddened and every test in THIS file still passed, for the relays under + * objectui#4044 (PR #9202) and for the dataset face under objectui#9203. * - * ⭐ What this file is NOT (objectui#4044). Since that card the two INLINE - * dashboard relays (`DashboardRenderer` and `DashboardGridLayout`, for widgets - * bound to inline rows or to a `provider: 'object'` aggregate rather than to an - * ADR-0021 dataset) lower the same keys through the same - * `chartConfigPresentation` whitelist onto a node of the same shape. It is - * tempting to read the assertions below as covering those relays too. They do - * not, and the difference was measured: with the forwarding deleted from BOTH - * relays, every test in this file still passed — because the schema above is - * hand-built here rather than composed by a relay. What this file pins is the - * CHART BLOCK: that a node carrying these keys draws them. The dashboard - * surface pins its own end of the chain, in plugin-dashboard's - * `DashboardChart.chartConfig-4044.test.tsx` (the seam) and its two - * end-to-end siblings `…chartConfigDom-4044` and `…chartConfigMarks-4044`. + * The reason is structural, so it will not change: the schema above is + * hand-built in this file rather than composed by a dashboard surface, so no + * dashboard forwarding is on the path these assertions execute. ⭐ An assertion + * that stays green while the thing it names is deleted is not evidence of it. + * What this file pins is the CHART BLOCK: that a node carrying these keys draws + * them. Each dashboard surface pins its own end of the chain, in + * plugin-dashboard: `DashboardChart.chartConfig-4044.test.tsx` (relay seam) + * with `…chartConfigDom-4044` and `…chartConfigMarks-4044`, and + * `DatasetWidget.chartConfig.test.tsx` (dataset seam) with + * `DatasetWidget.chartConfig.dom.test.tsx` and + * `DatasetWidget.chartConfigMarks-9203.test.tsx`. */ import React from 'react'; diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx index 9da9dd4673..14a70d20dc 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx @@ -11,15 +11,41 @@ * (`ChartRenderer`) → `AdvancedChartImpl` — with no renderer stub at all, and * reads the resulting DOM. * - * Scope of what can be proven here: everything the chart draws OUTSIDE Recharts' + * Scope of THIS file: everything the chart draws OUTSIDE Recharts' * `ResponsiveContainer` — the ChartFrame titles, and the chart container's * height and accessible name. Recharts' own marks (bars, LabelList, reference - * lines, Brush) need a measured box, which the headless DOM never provides + * lines, Brush) need a measured box, which this harness does not give them * (`ResponsiveContainer` measures 0×0 and renders no children), so asserting - * their absence *here* would pass for the wrong reason. Those keys are pinned in - * `packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx`, - * which mocks `ResponsiveContainer` to a fixed size — the only place in this - * repo that can, since `recharts` resolves inside plugin-charts alone. + * their absence *here* would pass for the wrong reason. + * + * ⚠️ This header used to end by saying those marks could be pinned in + * `packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx` + * alone, "the only place in this repo that can, since `recharts` resolves + * inside plugin-charts alone". objectui#9203 corrected that, and both halves + * of the correction matter: + * + * - The premise survives — `recharts` does resolve inside `plugin-charts` + * alone, so `vi.mock('recharts')` is genuinely unavailable here. + * - ⭐ The conclusion was false, and worse than false: that file hand-builds + * its own chart schema, so it never travels the dashboard seam at all. When + * PR objectui#9202 ablated the inline relays' forwarding, **46 assertions + * went red across the three dashboard files and 0 in that one**. It was + * never coverage for the plot-internal keys on any dashboard face — an + * assertion that stays green while the thing it names is deleted is not + * evidence (objectui#7963's `confirmVariant`, same shape). + * + * No module mock is needed to close that: `ResponsiveContainer` seeds its size + * from `getBoundingClientRect` on its OWN element, so a stub scoped to the + * `recharts-responsive-container` element is enough. The dataset face's + * plot-internal keys (`colors`, `categoryColors`, `showDataLabels`, + * `annotations`, `interaction`) are pinned that way, on this seam, in + * `DatasetWidget.chartConfigMarks-9203.test.tsx` — the sibling to this file. + * ⛔ Do not cite `plugin-charts`' file as their coverage again. + * + * The assertions below are deliberately left exactly as they were: they are the + * LIT CONTROL for that ablation (the four keys that do redden here), and this + * file's own harness stays box-free so the negative `aria` pin at the bottom + * keeps meaning what it says. */ import { describe, it, expect, vi, afterEach } from 'vitest'; diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx index 02d5969c64..f32133dc3e 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx @@ -10,10 +10,16 @@ * * - forwarded, because the chart block measurably draws it (the DOM half of * that claim lives in `DatasetWidget.chartConfig.dom.test.tsx` for the keys - * drawn outside the plot and in plugin-charts' - * `ChartRenderer.dashboardChartConfig.test.tsx` for the ones drawn inside it): + * drawn outside the plot and in `DatasetWidget.chartConfigMarks-9203.test.tsx` + * for the ones drawn inside it): * `title`, `subtitle`, `description`, `height`, `colors`, `showDataLabels`, * `annotations`, `interaction` — beside the pre-existing `showLegend`; + * + * ⛔ That second citation used to name plugin-charts' + * `ChartRenderer.dashboardChartConfig.test.tsx`, and it was wrong: + * objectui#9203 deleted this face's forwarding and that file did not redden, + * because it hand-builds its own chart schema and never travels this seam. + * ⛔ Do not cite it as this face's coverage again — see its own header. * - refused, because the value is DERIVED from the dataset selection and an * authored one would shadow it: `type`, and the BINDING keys inside * `xAxis`/`yAxis`/`series` (`ChartAxis.field`, `ChartSeries.name`); diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfigMarks-9203.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfigMarks-9203.test.tsx new file mode 100644 index 0000000000..289937ad3b --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfigMarks-9203.test.tsx @@ -0,0 +1,215 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#9203 — the PLOT-INTERNAL half of the DATASET face's `chartConfig` + * forwarding, asserted **on the dashboard surface**: dataset widget metadata + * in, drawn Recharts marks out, no renderer stub anywhere in the chain. + * + * ## Why this file exists beside `DatasetWidget.chartConfig.dom.test.tsx` + * + * objectui#4044's ruling names the pin shape: every forwarded key gets a + * rendering assertion *on the dashboard surface* that reads it — never a "the + * prop was passed" assertion. On this face that clause was half done. + * `DatasetWidget.chartConfig.test.tsx` pins the seam (which keys + * `chartConfigPresentation` composes) and `DatasetWidget.chartConfig.dom.test.tsx` + * pins the four keys that paint OUTSIDE Recharts' `ResponsiveContainer` + * (`title` / `subtitle` / `description` / `height`). The other five — + * `colors`, the `categoryColors` arm of `colors`, `showDataLabels`, + * `annotations` and `interaction` — paint *inside* it and had no + * dashboard-surface pin at all. + * + * ## ⭐ Why `plugin-charts`' file was never their coverage — measured + * + * Their only apparent evidence was + * `plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx`, which + * hand-builds its own chart schema and therefore never travels this seam. PR + * objectui#9202 ablated the forwarding out of the two inline relays and ran + * everything: **46 assertions red across the three dashboard files, and 0 in + * `plugin-charts`' file.** An assertion that stays green while the thing it + * names is deleted is not evidence (the shape objectui#7963's `confirmVariant` + * survived in for months). ⛔ That file is not coverage for these five keys. + * + * ## How the plot gets a box here, and why it is not a `recharts` mock + * + * `recharts` resolves inside `plugin-charts` alone, so `vi.mock('recharts')` is + * not available in this package. It is also not needed — the conclusion the + * sibling file drew from that ("those marks can only be pinned inside + * plugin-charts") is false, and this file is the counter-example. + * `ResponsiveContainer` reads `containerRef.current.getBoundingClientRect()` + * SYNCHRONOUSLY inside its resize effect and seeds its size from that, + * consulting `ResizeObserver` only for later changes — and the repo's + * happy-dom `ResizeObserver` polyfill is a no-op, which is exactly why nothing + * painted before. Sizing that one element is enough. + * + * ⚠️ SCOPED to the container element, never blanket — the blanket form looks + * like it works: with every element answering 480x320, Recharts' own axis-label + * measurement reads 480x320 too, the x-axis claims the whole box, and the plot + * clip rect comes back `height="0"` with the marks absent while + * `.recharts-surface` is present. A file that waited on the surface and then + * asserted "no data labels" would have passed for that reason. + * + * ⚠️ Every negative arm below is paired with a positive one taken through the + * SAME harness. A bare "no mark drawn" reading cannot tell a honoured + * `false` from a plot that never painted. + */ + +import { describe, it, expect, vi, afterEach, beforeAll, afterAll } from 'vitest'; +import { render, cleanup, screen, waitFor } from '@testing-library/react'; +// Registers `chart` in the ComponentRegistry, which is what the `SchemaRenderer` +// inside `DatasetWidget` resolves its `{ type: 'chart' }` schema through. +// Production reaches `AdvancedChartImpl` only through the `React.lazy(() => +// import('./AdvancedChartImpl'))` factory inside `ChartRenderer`, so this test +// reaches it the same way — by rendering the real chain and awaiting the +// Suspense boundary (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 = [ + { status: 'open', total: 120 }, + { status: 'paid', total: 80 }, +]; + +/** + * Render one DATASET-BOUND chart widget — `dataset` + `dimensions` + `values` + * resolved through `queryDataset`, which is the whole point of this face — and + * settle it at the drawn plot (`.recharts-surface`, not the chart container), + * so every assertion below reads a plot that really painted. An empty result + * then means "the chart drew and chose not to", never "it had not drawn yet". + */ +const renderWidget = async ( + chartConfig?: Record, + widgetType: 'bar' | 'pie' = 'bar', +) => { + const src = { queryDataset: vi.fn(async () => ({ rows })) }; + const view = render( + , + ); + // AGENTS.md records first-`import()` latencies up to 976 ms under full + // parallelism, past RTL's 1000 ms default once the recharts graph is cold. + await waitFor(() => expect(view.container.querySelector('.recharts-surface')).not.toBeNull(), { + timeout: 15000, + }); + return view; +}; + +const sectorFills = (c: HTMLElement) => + Array.from(c.querySelectorAll('path.recharts-sector')).map((p) => p.getAttribute('fill')); +// Measured, not assumed: with no `colors` on the schema the chart block falls +// back to the theme's own `--chart-N` ramp. Spelled out here so the control +// above reads a REAL palette rather than "whatever came back". +const DEFAULT_SECTOR_FILLS = ['hsl(var(--chart-1))', 'hsl(var(--chart-2))']; +const dataLabels = (c: HTMLElement) => + Array.from(c.querySelectorAll('.recharts-label-list text')).map((t) => t.textContent); + +describe('DatasetWidget — chartConfig reaches the drawn marks (objectui#9203)', () => { + it('paints the marks from an array `colors` palette', async () => { + // A pie draws one mark per CATEGORY, so a positional palette is readable + // straight off the sectors' fills — a bar's fill is a gradient `url(#…)`. + const { container } = await renderWidget({ colors: ['#111111', '#222222'] }, 'pie'); + expect(sectorFills(container)).toEqual(['#111111', '#222222']); + }); + + it('keeps the renderer default palette when `colors` is undeclared', async () => { + // The control for the case above, and the reason it is not vacuous: some + // palette always paints, so "the author's palette won" has to be told apart + // from "a palette was used". + const { container } = await renderWidget(undefined, 'pie'); + expect(sectorFills(container)).toEqual(DEFAULT_SECTOR_FILLS); + }); + + it('paints per-category colours from a record `colors` map', async () => { + // The record arm arrives at the renderer as `categoryColors` (the whitelist + // splits it) and wins per category — the precedence the spec's own `colors` + // field states. + const { container } = await renderWidget({ colors: { open: '#10B981', paid: '#EF4444' } }, 'pie'); + expect(sectorFills(container)).toEqual(['#10B981', '#EF4444']); + }); + + it('prints each point value on the mark when showDataLabels is on', async () => { + const { container } = await renderWidget({ showDataLabels: true }); + expect(dataLabels(container)).toEqual(['120', '80']); + }); + + it('prints no data labels when showDataLabels is off or undeclared', async () => { + const { container: off } = await renderWidget({ showDataLabels: false }); + expect(dataLabels(off)).toEqual([]); + cleanup(); + const { container: bare } = await renderWidget(); + expect(dataLabels(bare)).toEqual([]); + }); + + it('draws a reference line for a line annotation', async () => { + const { container } = await renderWidget({ + annotations: [{ type: 'line', axis: 'y', value: 100, label: 'Target' }], + }); + expect(container.querySelectorAll('.recharts-reference-line').length).toBeGreaterThan(0); + expect(screen.getByText('Target')).toBeTruthy(); + }); + + it('draws a reference area for a region annotation', async () => { + const { container } = await renderWidget({ + annotations: [{ type: 'region', axis: 'y', value: 50, endValue: 100 }], + }); + expect(container.querySelectorAll('.recharts-reference-area').length).toBeGreaterThan(0); + }); + + it('draws no reference marks when no annotation is declared', async () => { + const { container } = await renderWidget(); + expect(container.querySelectorAll('.recharts-reference-line').length).toBe(0); + expect(container.querySelectorAll('.recharts-reference-area').length).toBe(0); + }); + + it('adds the range selector when interaction.brush is on, and not by default', async () => { + const { container: on } = await renderWidget({ interaction: { brush: true } }); + expect(on.querySelectorAll('.recharts-brush').length).toBeGreaterThan(0); + cleanup(); + const { container: bare } = await renderWidget(); + expect(bare.querySelectorAll('.recharts-brush').length).toBe(0); + }); + + it('removes the hover tooltip when interaction.tooltips is false', async () => { + // The "on" arm is the control: without it a missing tooltip wrapper would + // read as honoured when it only meant the plot had not drawn. + const { container: on } = await renderWidget(); + expect(on.querySelectorAll('.recharts-tooltip-wrapper').length).toBeGreaterThan(0); + cleanup(); + const { container: off } = await renderWidget({ interaction: { tooltips: false } }); + expect(off.querySelectorAll('.recharts-tooltip-wrapper').length).toBe(0); + }); +});