From b7502b872aa06dcfde1c545b1d5146df092adc4f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 09:44:35 +0000 Subject: [PATCH] feat(plugin-dashboard): forward the chartConfig presentation keys the chart block delivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DashboardWidgetSchema.chartConfig` is declared as the full spec `ChartConfigSchema`, but the ADR-0021 dataset path lowered exactly one key onto the chart renderer — `showLegend` (#3135). `title`, `subtitle`, `description`, `height`, `colors`, `showDataLabels`, `annotations` and `interaction` parsed as valid metadata, reached DatasetWidget, and were dropped before the chart schema was built, even though the underlying chart block draws every one of them. DatasetWidget now lowers them, on two mechanical criteria that both have to hold: the chart block draws the key end to end on this path (each is pinned at the DOM level, so nothing is forwarded to a prop that ignores it), and the key does not fight the dataset derivation. `xAxis`/`yAxis`/`series` therefore stay out (derived from the dataset selection; an authored one would shadow the derived binding), `type` stays out (the widget's own type picks the family via CHART_TYPE_MAP), and `aria` stays out because nothing on this path reads it — AdvancedChartImpl has no `aria` prop and SchemaRenderer's ARIA injection reads the flat ariaLabel/ariaDescribedBy/role. `colors` is split the way the react tier splits it: a string[] is the positional palette, a {value: color} record is a per-category map merged over the category dimension's own option colours. Part of objectstack-ai/objectstack#5175 Fixes objectstack-ai/objectstack#7016 Co-Authored-By: Claude --- ...-widget-chartconfig-presentation-os7016.md | 51 +++++ ...hartRenderer.dashboardChartConfig.test.tsx | 216 ++++++++++++++++++ packages/plugin-dashboard/package.json | 1 + .../plugin-dashboard/src/DatasetWidget.tsx | 118 +++++++++- .../DatasetWidget.chartConfig.dom.test.tsx | 128 +++++++++++ .../DatasetWidget.chartConfig.test.tsx | 214 +++++++++++++++++ pnpm-lock.yaml | 3 + 7 files changed, 719 insertions(+), 12 deletions(-) create mode 100644 .changeset/dataset-widget-chartconfig-presentation-os7016.md create mode 100644 packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx create mode 100644 packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx create mode 100644 packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx diff --git a/.changeset/dataset-widget-chartconfig-presentation-os7016.md b/.changeset/dataset-widget-chartconfig-presentation-os7016.md new file mode 100644 index 0000000000..a4c8f6071c --- /dev/null +++ b/.changeset/dataset-widget-chartconfig-presentation-os7016.md @@ -0,0 +1,51 @@ +--- +"@object-ui/plugin-dashboard": patch +--- + +Dashboard metadata's `chartConfig` presentation keys now take effect for the first time + +`DashboardWidgetSchema.chartConfig` is declared as the full spec +`ChartConfigSchema`, but the ADR-0021 dataset path lowered exactly one key onto +the chart renderer: `showLegend` (objectui#3135). Everything else an author wrote +there — the chart's own `title`/`subtitle`, the accessibility `description`, an +explicit plot `height`, a `colors` palette or per-category colour map, +`showDataLabels`, `annotations`, `interaction` — parsed as valid metadata, +reached `DatasetWidget`, and was dropped before the chart schema was built. The +underlying chart block draws all of them; only the dashboard's hand-off was +missing. + +`DatasetWidget` now lowers each of those keys, on two mechanical criteria, both +of which have to hold: + +1. **The chart block draws it end to end on this path.** `{ type: 'chart' }` + resolves to `ChartRenderer` → `AdvancedChartImpl`, which draws + `title`/`subtitle` above the plot, turns `description` into the chart + container's `role="img"` + `aria-label`, applies `height` as that container's + inline height, paints `colors`, prints `showDataLabels` as per-point labels, + draws `annotations` as reference lines/bands and honours `interaction` as the + tooltip toggle plus the range selector. Each is pinned at the DOM level, so a + key is never forwarded to a prop that ignores it. +2. **It does not fight the dataset derivation.** `xAxis`, `yAxis` and `series` + are derived from the widget's dataset selection, so an authored one would + shadow the derived binding and blank the chart; they stay unforwarded, as does + `type` (the widget's own `type` already picks the chart family). `aria` stays + unforwarded too, for the other reason: nothing on this path reads it. + +`colors` is split the way the react tier already splits it, because the two arms +reach the renderer through different props: a `string[]` is the positional +palette, a `{ value: color }` record is a per-category map merged over the +category dimension's own option colours. + +**Behaviour-opening surface.** A dashboard that already wrote any of these keys +goes from having them ignored to having them applied — the point of the change, +but visible: a widget that declared `chartConfig.title` now shows that title +inside the plot area (in addition to the widget card's own `title`, which is a +separate key), one that declared `height` no longer fills its card, one that +declared `colors` stops using the theme palette, and `showDataLabels`, +`annotations` and `interaction.brush` start drawing. Widgets with no +`chartConfig`, or with only `showLegend`, render exactly as before: undeclared +keys are never emitted, so the renderer's own defaults stay in charge. + +Part of objectstack#5175 (the enforce half); the narrowing half — what to do +about `aria`, and about `xAxis`/`yAxis`/`series` being declared on a surface that +derives them — is still open there. diff --git a/packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx b/packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx new file mode 100644 index 0000000000..3b527da70c --- /dev/null +++ b/packages/plugin-charts/src/ChartRenderer.dashboardChartConfig.test.tsx @@ -0,0 +1,216 @@ +/** + * 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. + */ + +/** + * objectstack#7016 — the plot-internal half of the dashboard `chartConfig` + * forwarding, pinned where it can actually be seen. + * + * `DatasetWidget` (plugin-dashboard) now lowers the `chartConfig` keys the chart + * block delivers onto the `{ type: 'chart' }` schema it hands to the renderer. + * The criterion for lowering a key is that the chart DRAWS it, so each one needs + * 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. + * + * These render `ChartRenderer`, not `AdvancedChartImpl`: `ChartRenderer` is what + * the ComponentRegistry resolves `type: 'chart'` to, so it is the component the + * dashboard path actually reaches, and the schema below is byte-for-byte the + * 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. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, screen, waitFor } from '@testing-library/react'; + +// Recharts' ResponsiveContainer measures via ResizeObserver, which reports 0×0 +// under the headless DOM, so nothing paints. Fix its size. +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +// `ChartRenderer` renders its implementation behind +// `React.lazy(() => import('./AdvancedChartImpl'))`. Importing it here — with the +// SAME specifier, so the ESM cache satisfies the lazy factory — pays the recharts +// graph in the import phase, which no test timeout applies to (AGENTS.md §测试纪律). +import './AdvancedChartImpl'; +import { ChartRenderer } from './ChartRenderer'; + +afterEach(cleanup); + +/** The implementation is lazy — wait for the real plot, not the skeleton. */ +const plotted = async (c: HTMLElement) => { + await waitFor(() => expect(c.querySelector('.recharts-surface')).toBeTruthy()); + return c; +}; + +/** + * The chart schema a dataset-bound dashboard widget emits for + * `type: 'bar'`, `dimensions: ['status']`, `values: ['total']` — the derived + * bindings only. Presentation keys are spread in per test, exactly as + * `DatasetWidget` spreads its `chartConfig` presentation over this object. + */ +const dashboardSchema = (presentation: Record = {}) => ({ + type: 'chart', + chartType: 'bar' as const, + data: [ + { status: 'open', total: 120 }, + { status: 'paid', total: 80 }, + ], + xAxisKey: 'status', + series: [{ dataKey: 'total', label: 'Total' }], + isAnimationActive: false, + ...presentation, +}); + +describe('dashboard chartConfig — colors (objectstack#7016)', () => { + const sectorFills = (container: HTMLElement) => + Array.from(container.querySelectorAll('path.recharts-sector')).map((p) => p.getAttribute('fill')); + + 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. + const { container } = render( + , + ); + expect(sectorFills(await plotted(container))).toEqual(['#111111', '#222222']); + }); + + it('paints per-category colours from a record `colors` map, over the palette', async () => { + // The record arm of `colors` arrives as `categoryColors` (DatasetWidget does + // the split) and wins per category, which is the precedence the spec's own + // `colors` field comment states. + const { container } = render( + , + ); + expect(sectorFills(await plotted(container))).toEqual(['#10B981', '#EF4444']); + }); +}); + +describe('dashboard chartConfig — showDataLabels (objectstack#7016)', () => { + const labelTexts = (container: HTMLElement) => + Array.from(container.querySelectorAll('.recharts-label-list text')).map((t) => t.textContent); + + it('prints each point value on the mark when on', async () => { + const { container } = render(); + expect(labelTexts(await plotted(container))).toEqual(['120', '80']); + }); + + it('prints no data labels when off or undeclared', async () => { + // `plotted` first: an empty label list has to mean "the plot drew and chose + // not to label", never "nothing rendered yet". + const { container: off } = render(); + expect(labelTexts(await plotted(off))).toEqual([]); + cleanup(); + const { container: bare } = render(); + expect(labelTexts(await plotted(bare))).toEqual([]); + }); +}); + +describe('dashboard chartConfig — annotations (objectstack#7016)', () => { + it('draws a reference line for a line annotation', async () => { + const { container } = render( + , + ); + await plotted(container); + 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 } = render( + , + ); + await plotted(container); + expect(container.querySelectorAll('.recharts-reference-area').length).toBeGreaterThan(0); + }); + + it('draws nothing extra when no annotation is declared', async () => { + const { container } = render(); + await plotted(container); + expect(container.querySelectorAll('.recharts-reference-line').length).toBe(0); + expect(container.querySelectorAll('.recharts-reference-area').length).toBe(0); + }); +}); + +describe('dashboard chartConfig — interaction (objectstack#7016)', () => { + it('adds the range selector when interaction.brush is on', async () => { + const { container } = render( + , + ); + await plotted(container); + expect(container.querySelectorAll('.recharts-brush').length).toBeGreaterThan(0); + }); + + it('omits the range selector by default', async () => { + const { container } = render(); + await plotted(container); + expect(container.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 } = render(); + await plotted(on); + expect(on.querySelectorAll('.recharts-tooltip-wrapper').length).toBeGreaterThan(0); + cleanup(); + const { container: off } = render( + , + ); + await plotted(off); + expect(off.querySelectorAll('.recharts-tooltip-wrapper').length).toBe(0); + }); +}); + +describe('dashboard chartConfig — the keys that stay out (objectstack#7016)', () => { + // `aria` is declared by ChartConfigSchema and read by NOTHING on this path, so + // DatasetWidget refuses to lower it. This pins the "read by nothing" half: even + // handed straight to the renderer the object changes no attribute, which is why + // forwarding it would only have moved declared-but-inert one layer down. + it('an `aria` object handed to the chart changes no attribute', async () => { + const { container } = render( + , + ); + await plotted(container); + const chart = container.querySelector('[data-slot="chart"]') as HTMLElement; + expect(chart.getAttribute('role')).toBeNull(); + expect(chart.getAttribute('aria-label')).toBeNull(); + expect(container.querySelector('[aria-describedby]')).toBeNull(); + }); +}); diff --git a/packages/plugin-dashboard/package.json b/packages/plugin-dashboard/package.json index 53f5b0e479..ddfc1a5f08 100644 --- a/packages/plugin-dashboard/package.json +++ b/packages/plugin-dashboard/package.json @@ -42,6 +42,7 @@ "react-grid-layout": "^2.2.0 || ^1.4.0" }, "devDependencies": { + "@object-ui/plugin-charts": "workspace:*", "@objectstack/spec": "^17.0.0-rc.5", "@types/react-grid-layout": "^2.1.0", "@vitejs/plugin-react": "^6.0.5", diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index 8e89df12ae..cde8e29169 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -331,6 +331,103 @@ const CHART_TYPE_MAP: Record = { sankey: 'sankey', }; +/** + * Lower a dashboard widget's declared `chartConfig` (spec `ChartConfigSchema` — + * the same shape a report block and a react `` parse) onto the + * chart schema this widget hands to the renderer. + * + * ## Why this is a whitelist and not a spread + * + * Until #3135 NONE of `chartConfig` reached the renderer: this widget read + * `options` and nothing else, so an author who wrote `showLegend: false` still + * got a legend and one who wrote `true` only got one because "on" is the + * renderer's default. #3135 lowered that single flag and left the rest declared + * and inert. objectstack#7016 lowers the rest of the keys that are actually + * DELIVERED, admitting a key only when both of these hold: + * + * 1. **The chart block draws it end to end on this path.** `{ type: 'chart' }` + * resolves to `ChartRenderer` → `AdvancedChartImpl`, which draws + * `title`/`subtitle` in its ChartFrame, turns `description` into the chart + * container's `role="img"` + `aria-label`, applies `height` as that + * container's inline height, reads `colors` as the positional palette, + * prints `showDataLabels` as a Recharts `LabelList`, draws `annotations` as + * ReferenceLine/ReferenceArea and honours `interaction` as the tooltip + * toggle plus `Brush`. Forwarding a key the renderer ignores would only + * move declared-but-not-delivered one layer down, which is the failure this + * change exists to remove. + * 2. **It does not fight the dataset derivation.** `xAxis` / `yAxis` / + * `series` are DERIVED from the dataset selection (`buildChartSeries`), so + * they stay unforwarded: an authored axis or series array would shadow the + * derived binding and blank the chart. `type` stays out for the same + * reason — the widget's own `type` already picks the family through + * `CHART_TYPE_MAP`, which is the dataset path's chart-family channel. + * + * `aria` is the one declared key with **no reader at all** on this path: + * `AdvancedChartImpl` has no `aria` prop, and `SchemaRenderer`'s ARIA injection + * reads the FLAT `ariaLabel`/`ariaDescribedBy`/`role`, never a nested `aria` + * object. It is therefore left unforwarded on purpose (criterion 1) and + * reported back to objectstack#5175's narrowing half rather than papered over + * with a dashboard-only flattening that would also collide with the accessible + * name `description` already sets. + * + * @param raw the widget's `chartConfig` as authored (anything, incl. absent) + * @param fieldCategoryColors per-category colours resolved from the category + * dimension's own select/lookup option colours, merged UNDER an explicit + * author map (see the `colors` note below) + * @returns only the keys that resolved, so the caller can spread it over the + * derived chart schema and every undeclared key keeps the renderer's default + */ +export function chartConfigPresentation( + raw: unknown, + fieldCategoryColors?: Record | null, +): Record { + const config: Record = + raw && typeof raw === 'object' && !Array.isArray(raw) ? (raw as Record) : {}; + const out: Record = {}; + + const text = (v: unknown): string | undefined => (typeof v === 'string' && v ? v : undefined); + + if (typeof config.showLegend === 'boolean') out.showLegend = config.showLegend; + if (typeof config.showDataLabels === 'boolean') out.showDataLabels = config.showDataLabels; + const title = text(config.title); + if (title) out.title = title; + const subtitle = text(config.subtitle); + if (subtitle) out.subtitle = subtitle; + const description = text(config.description); + if (description) out.description = description; + // A non-positive height would collapse the plot; the container default is the + // more honest answer than an invisible chart. + if (typeof config.height === 'number' && Number.isFinite(config.height) && config.height > 0) { + out.height = config.height; + } + if (Array.isArray(config.annotations) && config.annotations.length > 0) out.annotations = config.annotations; + if (config.interaction && typeof config.interaction === 'object' && !Array.isArray(config.interaction)) { + out.interaction = config.interaction; + } + + // `colors` is overloaded kanban-style — and the two arms reach the renderer + // through two DIFFERENT props, so the split has to happen here (the react + // tier's ObjectChart splits it the same way): a `string[]` is the positional + // palette (`colors`), a `{ value: color }` record is an explicit per-category + // map (`categoryColors`). The author's map is merged OVER the dimension + // field's own option colours, which is the precedence the spec field comment + // states ("a value→color map — and a select/lookup dimension's option colors + // — take precedence over the positional palette per category"). + const palette = Array.isArray(config.colors) + ? config.colors.filter((c): c is string => typeof c === 'string' && !!c) + : undefined; + if (palette?.length) out.colors = palette; + const authorCategoryColors = + config.colors && typeof config.colors === 'object' && !Array.isArray(config.colors) + ? (config.colors as Record) + : undefined; + if (fieldCategoryColors || authorCategoryColors) { + out.categoryColors = { ...(fieldCategoryColors ?? {}), ...(authorCategoryColors ?? {}) }; + } + + return out; +} + export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: unknown }) { const datasetName = String(widget?.dataset ?? ''); const dimensions: string[] = useMemo(() => (Array.isArray(widget?.dimensions) ? widget.dimensions.filter(Boolean) : []), [widget]); @@ -984,17 +1081,14 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: }); const effectiveCategoryOrder = explicitOrder?.length ? explicitOrder : categoryOrder; - // `chartConfig.showLegend` (#3135). The widget's chart config never reached - // the renderer — this component read `options` and nothing else — so an author - // who wrote `showLegend: false` still got a legend, and one who wrote `true` - // only got one because "on" happens to be the renderer's default. Lower the - // one flag the renderer already honors; the rest of `chartConfig` stays - // unforwarded (the renderer derives axes/series from the dataset selection). - const chartConfig: Record = - widget?.chartConfig && typeof widget.chartConfig === 'object' && !Array.isArray(widget.chartConfig) - ? (widget.chartConfig as Record) - : {}; - const showLegend = typeof chartConfig.showLegend === 'boolean' ? chartConfig.showLegend : undefined; + // The widget's declared `chartConfig`, lowered onto the chart schema — + // #3135 for `showLegend`, objectstack#7016 for the rest of the keys the chart + // block measurably delivers. See `chartConfigPresentation` for the two + // criteria a key has to meet and for why `xAxis`/`yAxis`/`series`/`type`/ + // `aria` are deliberately NOT here. It also owns the `colors` split, so the + // per-category map it returns already carries the dimension field's own + // option colours underneath any explicit author map. + const chartPresentation = chartConfigPresentation(widget?.chartConfig, categoryColors); // Map a clicked chart segment back to its dataset row, then drill through to // the underlying records — same governed path the table/pivot rows use. @@ -1021,7 +1115,7 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: // measurement churn, can freeze there — bars never draw until an unrelated // re-render (#2756, follow-up to #2727's ineffective settle re-mount). // Turning the tween off makes the first paint deterministic. - schema={{ type: 'chart', chartType, data: chartData, xAxisKey, series: chartSeries, isAnimationActive: false, ...(showLegend != null ? { showLegend } : {}), ...(categoryColors ? { categoryColors } : {}), ...(effectiveCategoryOrder ? { categoryOrder: effectiveCategoryOrder } : {}) } as any} + schema={{ type: 'chart', chartType, data: chartData, xAxisKey, series: chartSeries, isAnimationActive: false, ...chartPresentation, ...(effectiveCategoryOrder ? { categoryOrder: effectiveCategoryOrder } : {}) } as any} onChartClick={chartDrill} onSegmentClick={chartDrill} /> diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx new file mode 100644 index 0000000000..fdaf5b0d0b --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.dom.test.tsx @@ -0,0 +1,128 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#7016 — END TO END: dashboard metadata → real chart DOM. + * + * The sibling `DatasetWidget.chartConfig.test.tsx` pins the decision at the + * schema seam. That is necessary and not sufficient: the criterion for lowering + * a `chartConfig` key is that the chart block *draws* it, and a seam assertion + * cannot tell a honoured prop from an ignored one. So this file renders the REAL + * chain — `DatasetWidget` → `SchemaRenderer` → the registry's `chart` + * (`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' + * `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 + * (`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. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, screen, waitFor } from '@testing-library/react'; + +// Registers `chart` in the ComponentRegistry, which is what `SchemaRenderer` +// resolves the widget's `{ type: 'chart' }` schema through. +import '@object-ui/plugin-charts'; +// `ChartRenderer` renders its implementation behind +// `React.lazy(() => import('./AdvancedChartImpl'))`, and every assertion below +// lives inside that Suspense boundary. Loading it is unbounded work — under full +// parallelism a first import of the recharts graph can outlast RTL's 1000ms +// `waitFor` window — so pay it in the import phase, which no test or hook +// timeout applies to (AGENTS.md §测试纪律). The alias maps this specifier to the +// very file the lazy factory imports, so the ESM cache already holds it. +import '@object-ui/plugin-charts/AdvancedChartImpl'; +import { DatasetWidget } from '../DatasetWidget'; + +afterEach(cleanup); + +const rows = [ + { status: 'open', total: 120 }, + { status: 'paid', total: 80 }, +]; + +const renderWidget = async (chartConfig?: Record) => { + const src = { queryDataset: vi.fn(async () => ({ rows })) }; + const view = render( + , + ); + // The chart container only exists once the dataset resolves AND the lazy chart + // chunk has mounted — i.e. once the whole dashboard chart path really ran. + await waitFor(() => expect(view.container.querySelector('[data-slot="chart"]')).not.toBeNull()); + return view; +}; + +const chartEl = (container: HTMLElement) => container.querySelector('[data-slot="chart"]') as HTMLElement; + +describe('DatasetWidget — chartConfig reaches the real chart DOM (objectstack#7016)', () => { + it('draws chartConfig.title / .subtitle above the plot', async () => { + const { container } = await renderWidget({ type: 'bar', title: 'Invoice value', subtitle: 'by status' }); + expect(screen.getByText('Invoice value')).toBeTruthy(); + expect(screen.getByText('by status')).toBeTruthy(); + // The titles are drawn in a FRAME wrapping the plot, so the chart container + // is no longer the widget root's direct child — the structural counterpart of + // the "no chrome" case below. + expect(chartEl(container).parentElement).not.toBe(container.firstElementChild); + }); + + it('adds no title chrome when chartConfig declares none', async () => { + const { container } = await renderWidget({ type: 'bar' }); + // ChartFrame is a passthrough with neither title nor subtitle, so the plot + // sits directly under the widget root — no wrapper, no empty header row. + expect(chartEl(container).parentElement).toBe(container.firstElementChild); + }); + + it('announces chartConfig.description as the chart graphic accessible name', async () => { + const { container } = await renderWidget({ type: 'bar', description: 'Invoice value by status' }); + expect(chartEl(container).getAttribute('role')).toBe('img'); + expect(chartEl(container).getAttribute('aria-label')).toBe('Invoice value by status'); + }); + + it('leaves the graphic unlabelled when no description is declared', async () => { + // Not the same as an empty one: role="img" with no name is worse for a + // screen reader than a plain div it can skip past. + const { container } = await renderWidget({ type: 'bar' }); + expect(chartEl(container).getAttribute('role')).toBeNull(); + expect(chartEl(container).getAttribute('aria-label')).toBeNull(); + }); + + it('applies chartConfig.height over the container default', async () => { + const { container } = await renderWidget({ type: 'bar', height: 420 }); + expect(chartEl(container).style.height).toBe('420px'); + }); + + it('keeps the container default height when none is declared', async () => { + const { container } = await renderWidget({ type: 'bar' }); + expect(chartEl(container).style.height).toBe(''); + expect(chartEl(container).className).toContain('h-[350px]'); + }); + + // Negative pin for the one key refused on criterion 1 (nothing reads it). This + // assertion is meaningful in this harness precisely because BOTH elements that + // could have carried the name render outside `ResponsiveContainer`: the chart + // container (which `description` does label, above) and the SchemaRenderer + // wrapper (which would pick up a FLATTENED `ariaLabel`). Written, still + // ignored — the current contract, pending objectstack#5175. + it('ignores chartConfig.aria — no accessible name appears anywhere', async () => { + const { container } = await renderWidget({ + type: 'bar', + aria: { ariaLabel: 'Authored name', role: 'figure' }, + }); + expect(container.querySelector('[aria-label]')).toBeNull(); + expect(container.querySelector('[role="figure"]')).toBeNull(); + expect(chartEl(container).getAttribute('role')).toBeNull(); + }); +}); diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx new file mode 100644 index 0000000000..ca46623c7f --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx @@ -0,0 +1,214 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#7016 — the DECISION TABLE for `DashboardWidget.chartConfig`, pinned. + * + * `chartConfig` is declared as the full spec `ChartConfigSchema`, but the + * dashboard dataset path only ever lowered `showLegend` (#3135). This file pins + * which of the remaining keys now reach the chart schema and which are refused, + * because the split is a contract, not an implementation detail: + * + * - 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): + * `title`, `subtitle`, `description`, `height`, `colors`, `showDataLabels`, + * `annotations`, `interaction` — beside the pre-existing `showLegend`; + * - refused, because the value is DERIVED from the dataset selection and an + * authored one would shadow it: `xAxis`, `yAxis`, `series`, `type`; + * - refused, because nothing on this path reads it: `aria`. + * + * The refusals are pinned as hard as the forwards. Today's behaviour for them is + * the contract until objectstack#5175's narrowing half rules on the shape, and a + * silent "improvement" here would pre-empt that decision. + * + * Asserted at the source — the schema handed to the renderer — via a stubbed + * SchemaRenderer, the same seam `DatasetWidget.showLegend`/`.animation` use. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; + +let lastChartSchema: any = null; + +vi.mock('@object-ui/react', async (importOriginal) => ({ + ...(await importOriginal>()), + SchemaRenderer: (props: any) => { + lastChartSchema = props.schema; + return null; + }, +})); + +import { DatasetWidget } from '../DatasetWidget'; + +afterEach(() => { + cleanup(); + lastChartSchema = null; +}); + +const rows = [ + { status: 'open', total: 120 }, + { status: 'paid', total: 80 }, +]; + +/** Render a dataset chart widget with the given `chartConfig` and settle it. */ +const renderWidget = async (chartConfig?: Record, widgetType = 'bar') => { + const src = { queryDataset: vi.fn(async () => ({ rows })) }; + render( + , + ); + await waitFor(() => expect(lastChartSchema).not.toBeNull()); +}; + +describe('DatasetWidget — chartConfig keys that ARE lowered (objectstack#7016)', () => { + it('forwards the chart titles and the accessibility description', async () => { + await renderWidget({ + type: 'bar', + title: 'Invoice value', + subtitle: 'by status', + description: 'Invoice value by status', + }); + expect(lastChartSchema.title).toBe('Invoice value'); + expect(lastChartSchema.subtitle).toBe('by status'); + expect(lastChartSchema.description).toBe('Invoice value by status'); + }); + + it('forwards an explicit plot height', async () => { + await renderWidget({ type: 'bar', height: 420 }); + expect(lastChartSchema.height).toBe(420); + }); + + // A non-positive height would collapse the plot to nothing; the container + // default is a more honest answer than an invisible chart, so the key is + // dropped rather than lowered. + it('drops a non-positive or non-numeric height', async () => { + await renderWidget({ type: 'bar', height: 0 }); + expect('height' in lastChartSchema).toBe(false); + cleanup(); + lastChartSchema = null; + await renderWidget({ type: 'bar', height: -10 }); + expect('height' in lastChartSchema).toBe(false); + }); + + it('forwards showDataLabels in both directions', async () => { + await renderWidget({ type: 'bar', showDataLabels: true }); + expect(lastChartSchema.showDataLabels).toBe(true); + cleanup(); + lastChartSchema = null; + await renderWidget({ type: 'bar', showDataLabels: false }); + expect(lastChartSchema.showDataLabels).toBe(false); + }); + + it('forwards annotations and the interaction toggles', async () => { + await renderWidget({ + type: 'bar', + annotations: [{ type: 'line', axis: 'y', value: 100, label: 'Target' }], + interaction: { tooltips: false, brush: true }, + }); + expect(lastChartSchema.annotations).toEqual([ + { type: 'line', axis: 'y', value: 100, label: 'Target' }, + ]); + expect(lastChartSchema.interaction).toEqual({ tooltips: false, brush: true }); + }); + + it('drops an empty annotations array instead of emitting a dead key', async () => { + await renderWidget({ type: 'bar', annotations: [] }); + expect('annotations' in lastChartSchema).toBe(false); + }); + + // `colors` is overloaded: a string[] is the positional palette, a + // { value: color } record is a per-category map. They reach the renderer + // through two DIFFERENT props, so the widget splits them — the same split the + // react tier's ObjectChart performs. + it('lowers an array `colors` as the positional palette', async () => { + await renderWidget({ type: 'bar', colors: ['#111111', '#222222'] }); + expect(lastChartSchema.colors).toEqual(['#111111', '#222222']); + expect('categoryColors' in lastChartSchema).toBe(false); + }); + + it('lowers a record `colors` as the per-category map, not as the palette', async () => { + await renderWidget({ type: 'pie', colors: { open: '#10B981', paid: '#EF4444' } }); + expect(lastChartSchema.categoryColors).toEqual({ open: '#10B981', paid: '#EF4444' }); + expect('colors' in lastChartSchema).toBe(false); + }); + + it('keeps the pre-existing showLegend behaviour (#3135)', async () => { + await renderWidget({ type: 'bar', showLegend: false }); + expect(lastChartSchema.showLegend).toBe(false); + }); + + // The whole point of a whitelist: an undeclared key leaves the renderer's own + // default in charge, so every dashboard that never wrote `chartConfig` renders + // byte-for-byte as before. + it('emits none of the presentation keys when no chartConfig is declared', async () => { + await renderWidget(); + for (const key of [ + 'title', 'subtitle', 'description', 'height', 'colors', 'categoryColors', + 'showLegend', 'showDataLabels', 'annotations', 'interaction', + ]) { + expect({ key, present: key in lastChartSchema }).toEqual({ key, present: false }); + } + }); +}); + +describe('DatasetWidget — chartConfig keys that are REFUSED (objectstack#7016)', () => { + // Criterion 2: the axes and the series are derived from the dataset selection + // (`buildChartSeries`). An authored `xAxis`/`yAxis`/`series` would shadow the + // derived binding inside `normalizeChartSchema` and blank the chart, so they + // are not lowered at all. This is the negative pin the issue asks for: the key + // is written, and it stays ignored. + it('ignores an authored xAxis / yAxis and keeps the derived axis binding', async () => { + await renderWidget({ + type: 'bar', + xAxis: { field: 'not_a_column', title: 'Authored X' }, + yAxis: [{ field: 'not_a_measure', min: 0, max: 5 }], + }); + expect('xAxis' in lastChartSchema).toBe(false); + expect('yAxis' in lastChartSchema).toBe(false); + // The derived binding is untouched: the dimension is still the category axis. + expect(lastChartSchema.xAxisKey).toBe('status'); + }); + + it('ignores an authored series and keeps one derived series per measure', async () => { + await renderWidget({ type: 'bar', series: [{ name: 'not_a_measure', stack: 'g' }] }); + // `series` on the emitted schema is the DERIVED one (internal `dataKey` + // shape, one entry per selected measure) — not the authored array. + expect(lastChartSchema.series).toHaveLength(1); + expect(lastChartSchema.series[0].dataKey).toBe('total'); + expect(lastChartSchema.series[0].name).toBeUndefined(); + expect(lastChartSchema.series[0].stack).toBeUndefined(); + }); + + it('ignores chartConfig.type — the widget type owns the chart family', async () => { + await renderWidget({ type: 'line' }, 'pie'); + expect(lastChartSchema.chartType).toBe('pie'); + // Neither the author `type` nor its rescued `specType` spelling is lowered, + // so nothing can outrank CHART_TYPE_MAP. + expect(lastChartSchema.type).toBe('chart'); + expect('specType' in lastChartSchema).toBe(false); + }); + + // Criterion 1: `aria` is declared by ChartConfigSchema and read by NOTHING on + // this path — AdvancedChartImpl has no `aria` prop, and SchemaRenderer's ARIA + // injection reads the FLAT `ariaLabel`/`ariaDescribedBy`/`role`. Forwarding it + // (nested, or flattened onto those three) would either stay inert or fight the + // accessible name `description` already sets. It stays out until #5175 rules. + it('ignores aria — nested and flattened', async () => { + await renderWidget({ + type: 'bar', + aria: { ariaLabel: 'Authored name', ariaDescribedBy: 'hint', role: 'figure' }, + }); + for (const key of ['aria', 'ariaLabel', 'ariaDescribedBy', 'role']) { + expect(key in lastChartSchema).toBe(false); + } + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5bfc0c1c4a..f902fc198f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1674,6 +1674,9 @@ importers: specifier: ^3.6.0 version: 3.6.0 devDependencies: + '@object-ui/plugin-charts': + specifier: workspace:* + version: link:../plugin-charts '@objectstack/spec': specifier: ^17.0.0-rc.5 version: 17.0.0-rc.5(ai@7.0.44(zod@4.4.3))