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))