diff --git a/.changeset/combo-dataset-path-presentation-merge-4229.md b/.changeset/combo-dataset-path-presentation-merge-4229.md new file mode 100644 index 000000000..6be6557e8 --- /dev/null +++ b/.changeset/combo-dataset-path-presentation-merge-4229.md @@ -0,0 +1,17 @@ +--- +'@object-ui/plugin-dashboard': patch +--- + +Dashboard `combo` widgets draw as combos on the dataset path — the dataset owns the data, the author owns the presentation + +A widget authoring the spec's own combo shape — `series[].type` plus `series[].yAxis: 'left'|'right'` and two `yAxis` entries — rendered as two bar series on one shared axis. Measured in the DOM: 2 bars, 0 lines, 1 y-axis, where 1 bar, 1 line and 2 axes were authored, so a percentage measure was plotted against a raw count's scale. + +Two halves caused it, and fixing either alone leaves a worse state than before. `CHART_TYPE_MAP` had no `combo` entry, so a `combo` widget fell through its `?? 'bar'` default — bars, whatever the series said. And `chartConfigPresentation` refused to forward `series` / `xAxis` / `yAxis` at all, on the stated grounds that they are derived from the dataset selection, so the per-series mark and the axis binding could never reach the renderer even once the family resolved. + +That belief was half right. The dataset does own the series MEMBERSHIP — which columns become series, which rows, which buckets — and it still does: an authored entry naming a measure the dataset did not select is ignored, and a derived series the author said nothing about keeps the family default. What the dataset never owned is the PRESENTATION carried on those same objects: the per-series mark, its left/right axis binding, label, colour, stack, and the axis definitions' title, format, min, max, step, grid and position. Those are the author's, and they now merge onto the derived bindings by name/key match with the explicit binding winning — one merge function, not a spread per attribute. The split runs through the two binding keys: `ChartSeries.name` and `ChartAxis.field` name a column and stay with the dataset; everything else on the object travels. + +This is objectui#2880's S2 rule, which PR #2883 landed in `ObjectChart` and which the dataset path never carried over. Dropping `ChartAxis.field` on the way through is what makes forwarding the axes safe rather than merely guarded: it is the one key by which an authored axis could have named a series, since the renderer synthesises series from `yAxis[].field` when a chart declares none. + +Two consequences beyond the reported bug. A non-combo widget can now declare one line series and get the combo the renderer already knew how to derive from disagreeing series types. And a `compareTo` overlay inherits its own measure's mark and axis, so the comparison of a bar-on-the-left measure no longer draws as a line on the right the moment the chart becomes a combo. + +Dashboards that never authored `chartConfig.series` or `chartConfig.yAxis` emit exactly what they emitted before. diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index c10a34702..1a7bc109c 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -51,6 +51,7 @@ import { pivotDimensionValue, pivotCellKey, compareToTrendLabelKey, + type ChartSeriesBinding, type CompareToConfig, type DatasetResultField, type DatasetDrillRange, @@ -317,6 +318,15 @@ const METRIC_TYPES = new Set(['metric', 'kpi', 'gauge', 'solid-gauge', 'bullet'] * relative (e.g. `spline`/`step-line` → line, `stacked-area` → area, * `pyramid` → funnel, grouped/stacked/bi-polar bars → bar) so a widget never * renders blank or as a misleading default. + * + * `combo` is NOT such a fallback: the renderer draws it distinctly (mixed + * marks on a `ComposedChart`, a left and a right y-axis), and it is a + * `ChartTypeSchema` member since spec 17.0.0-rc.1 — so it maps to ITSELF. + * Until #4229 it had no entry at all and fell through the `?? 'bar'` default, + * which is one of the two halves that made an authored combo render as grouped + * bars; the other half is the presentation merge below. `widgetDispatch` + * already resolves a `combo` widget to `chartType: 'combo'` + * (`SERIES_CHART_TYPES`), so this entry makes the two surfaces agree. */ const CHART_TYPE_MAP: Record = { bar: 'bar', @@ -339,6 +349,7 @@ const CHART_TYPE_MAP: Record = { radar: 'radar', treemap: 'treemap', sankey: 'sankey', + combo: 'combo', }; /** @@ -365,12 +376,40 @@ const CHART_TYPE_MAP: Record = { * 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. + * 2. **It does not fight the dataset derivation.** `type` stays out: the + * widget's own `type` already picks the family through `CHART_TYPE_MAP`, + * which is the dataset path's chart-family channel. + * + * ## Where `xAxis` / `yAxis` / `series` go — the ruled split (#4229) + * + * Those three used to be refused here under the same criterion 2, on the + * grounds that they are "DERIVED from the dataset selection". That belief was + * **half right, and the half it got wrong silently dropped authored intent**: + * a widget authoring the spec's own combo shape — `series[].type` plus + * `series[].yAxis: 'left'|'right'` and two `yAxis` entries — rendered as + * grouped bars on one axis, because the per-series mark and the axis binding + * never left this function (#4229, measured in the DOM: 2 bars / 0 lines / 1 + * axis where 1 bar + 1 line + 2 axes were authored). + * + * The ruling: **the dataset owns DATA, the author owns PRESENTATION.** + * + * - **Data (derived, never forwarded)** — series MEMBERSHIP (which columns + * become series, which rows, which buckets) and the column each binding + * reads. Concretely: `buildChartSeries`'s `dataKey`s, `xAxisKey`, and the + * spec's two binding keys `series[].name` and `ChartAxis.field`. + * - **Presentation (authored, merged forward)** — everything else on those + * same objects: `series[].type` (the per-series mark), `series[].yAxis` + * (which axis it binds to), `label`/`color`/`stack`/`variant`/`dashArray`/ + * `opacity`, and the axis definitions' `title`/`format`/`min`/`max`/ + * `stepSize`/`showGridLines`/`position`/`logarithmic`. + * + * This is #2880's S2 rule — dual axes are `yAxis[].position` plus + * `series[].yAxis`, and a combo assigns its axes by EXPLICIT binding first, + * falling back to the per-series-type guess only where the author bound + * nothing — extended from `ObjectChart` (where PR #2883 landed it) to the + * dataset path, which never carried it over. {@link mergeAuthoredPresentation} + * is the ONE place that merge happens; see it for the match rule and for why + * membership is safe. * * `aria` is the one declared key with **no reader at all** on this path: * `AdvancedChartImpl` has no `aria` prop, and `SchemaRenderer`'s ARIA injection @@ -438,6 +477,166 @@ export function chartConfigPresentation( return out; } +/** Authored spec `ChartSeries` presentation, in the renderer's internal spelling. */ +export interface AuthoredSeriesPresentation { + label?: string; + /** Spec `ChartSeries.type`, narrowed — see {@link seriesPresentation}. */ + chartType?: 'bar' | 'line' | 'area'; + yAxis?: 'left' | 'right'; + color?: string; + stack?: string; + variant?: 'primary' | 'comparison'; + dashArray?: string; + opacity?: number; +} + +/** A derived series binding with the author's presentation merged onto it. */ +export type MergedChartSeries = ChartSeriesBinding & AuthoredSeriesPresentation; + +const isRecord = (v: unknown): v is Record => + !!v && typeof v === 'object' && !Array.isArray(v); + +/** + * An i18n label is a plain string or a `{ en, zh-CN, … }` record; charts render + * a string. Same pick `normalizeChartSchema` makes, so a label reads the same + * on both paths. + */ +function labelText(v: unknown): string | undefined { + if (typeof v === 'string' && v) return v; + if (isRecord(v)) { + const first = Object.values(v).find((x) => typeof x === 'string' && x); + return first as string | undefined; + } + return undefined; +} + +/** + * One authored `ChartSeries`, minus its `name` — i.e. everything about it that + * is presentation rather than membership. + * + * `type` is narrowed to the three families that COMPOSE on one cartesian plot, + * because this array reaches the renderer already speaking the internal shape + * (`ChartRenderer` forwards a `dataKey`-shaped array untouched, so + * `normalizeChartSchema`'s own identical narrowing never sees it). Without the + * narrowing a `type: 'pie'` would not merely be inert — it would count as a + * family disagreement in `effectiveChartFamily`, flip the whole chart into a + * combo, and then draw that series as a bar anyway. + */ +function seriesPresentation(raw: Record): AuthoredSeriesPresentation { + const out: AuthoredSeriesPresentation = {}; + const family = raw.type; + if (family === 'bar' || family === 'line' || family === 'area') out.chartType = family; + if (raw.yAxis === 'left' || raw.yAxis === 'right') out.yAxis = raw.yAxis; + const label = labelText(raw.label); + if (label) out.label = label; + if (typeof raw.color === 'string' && raw.color) out.color = raw.color; + if (typeof raw.stack === 'string' && raw.stack) out.stack = raw.stack; + if (raw.variant === 'primary' || raw.variant === 'comparison') out.variant = raw.variant; + if (typeof raw.dashArray === 'string' && raw.dashArray) out.dashArray = raw.dashArray; + if (typeof raw.opacity === 'number' && Number.isFinite(raw.opacity)) out.opacity = raw.opacity; + return out; +} + +/** + * One authored `ChartAxis`, minus its `field` — the axis's presentation. + * + * `field` is the one DATA key on an axis (it names the plotted column), and + * dropping it here is what keeps membership with the dataset **structurally** + * rather than by a guard: `normalizeChartSchema` synthesises series out of + * `yAxis[].field` when a chart declares no series, so a forwarded `field` + * would be a live membership channel on an empty selection. With it gone the + * axis carries scale and chrome only, and the count of entries — which is what + * turns on the secondary axis (`yAxes.length > 1`) — survives, including for + * an entry that declares nothing but its own existence. + * + * Keys the renderer does not read on a given axis are dropped by + * `normalizeChartSchema`, the ONE normalization layer (#2880 S1): today it + * keeps `format`/`title`/`showGridLines` on the x-axis and the full set on the + * y-axes. That narrowing is deliberately NOT mirrored here — a second copy + * would drift from the renderer's real capability the moment it grew. + */ +function axisPresentation(raw: unknown): Record { + const out: Record = {}; + if (!isRecord(raw)) return out; + const title = labelText(raw.title); + if (title) out.title = title; + if (typeof raw.format === 'string' && raw.format) out.format = raw.format; + if (typeof raw.min === 'number' && Number.isFinite(raw.min)) out.min = raw.min; + if (typeof raw.max === 'number' && Number.isFinite(raw.max)) out.max = raw.max; + if (typeof raw.stepSize === 'number' && Number.isFinite(raw.stepSize) && raw.stepSize > 0) { + out.stepSize = raw.stepSize; + } + if (typeof raw.showGridLines === 'boolean') out.showGridLines = raw.showGridLines; + if (raw.position === 'left' || raw.position === 'right' || raw.position === 'top' || raw.position === 'bottom') { + out.position = raw.position; + } + if (typeof raw.logarithmic === 'boolean') out.logarithmic = raw.logarithmic; + return out; +} + +/** + * Merge the authored `chartConfig`'s PRESENTATION onto the series and axes the + * dataset selection derived — the one place that happens (#4229). + * + * The match rule is **by name/key**: an authored `series[].name` is paired with + * the derived binding whose `dataKey` it equals, and the pairing decides + * nothing but presentation: + * + * - an authored entry naming a measure that is NOT in the dataset selection is + * **ignored** — membership belongs to the dataset, so an author cannot add, + * remove or re-point a series from `chartConfig`; + * - a derived series with no authored entry keeps the family default, so every + * dashboard that never wrote `chartConfig.series` renders byte-for-byte as + * before; + * - where both exist the **explicit binding wins** (#2880 S2), which is the + * whole point: `type: 'line'` + `yAxis: 'right'` is how the spec says "this + * measure is a line on the secondary axis". + * + * Matching on `name` only is deliberate: `name` is the spec's authorable key + * for a series (`dataKey` is a declared ALIAS of it, resolved where the + * metadata is parsed), so reading a second spelling here would fossilize a + * dialect this renderer has no business accepting (AGENTS.md #0.1). + * + * @param derived the bindings `buildChartSeries` produced from the selection + * @param raw the widget's `chartConfig` as authored (anything, incl. absent) + * @returns the merged series, plus the presentation-only axes to spread onto + * the chart schema (absent when the author declared none) + */ +export function mergeAuthoredPresentation( + derived: ChartSeriesBinding[], + raw: unknown, +): { series: MergedChartSeries[]; axes: Record } { + const config: Record = isRecord(raw) ? raw : {}; + + const authored = new Map>(); + for (const entry of Array.isArray(config.series) ? config.series : []) { + if (!isRecord(entry)) continue; + const name = typeof entry.name === 'string' ? entry.name : undefined; + // First entry wins for a duplicated name — a later one cannot silently + // reconfigure a series the author already described. + if (name && !authored.has(name)) authored.set(name, entry); + } + const series: MergedChartSeries[] = derived.map((s) => { + const entry = authored.get(s.dataKey); + return entry ? { ...s, ...seriesPresentation(entry) } : s; + }); + + const axes: Record = {}; + const xAxis = axisPresentation(config.xAxis); + if (Object.keys(xAxis).length > 0) axes.xAxis = xAxis; + // The COUNT of y-axis entries is itself presentation — it is what declares a + // secondary axis — so every declared entry keeps its slot even when it + // carries nothing but `field` (which is data and does not travel). + const yAxisRaw = Array.isArray(config.yAxis) + ? config.yAxis + : config.yAxis !== undefined + ? [config.yAxis] + : []; + if (yAxisRaw.length > 0) axes.yAxis = yAxisRaw.map(axisPresentation); + + return { series, axes }; +} + 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]); @@ -1151,6 +1350,11 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: // series so multi-dimension dataset widgets match the chart-view renderer. const { data: chartData, xAxisKey, series } = buildChartSeries(chartRows, dimensions, values, state.fields); + // The author's PRESENTATION, merged onto those derived bindings — per-series + // mark and axis binding, plus the axis definitions (#4229). Membership stays + // with the dataset; see `mergeAuthoredPresentation` for the ruled split. + const { series: presentedSeries, axes: authoredAxes } = mergeAuthoredPresentation(series, widget?.chartConfig); + // Comparison overlay — one extra series per compared measure, carrying the // same `variant: 'comparison'` the inline chart's overlay uses (ObjectChart's // augmentedSeries), so AdvancedChartImpl draws it muted/dashed here too. @@ -1161,14 +1365,26 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: const pivotedSeries = dimensions.length >= 2 && values.length === 1; const comparisonSeries = pivotedSeries ? [] - : comparedValues.map((m) => ({ - dataKey: compareColumn(m), - label: `${headerLabel(m)} · ${compareLabel}`, - variant: 'comparison' as const, - })); + : comparedValues.map((m) => { + // An overlay is the SAME measure one period back, so it takes its + // primary's mark and axis — read off the already-merged series, never + // re-read from `chartConfig` (one merge path). Without this a combo's + // overlay fell to the renderer's positional guess and drew a bar + // measure as a line on the opposite axis. `stack` is deliberately NOT + // inherited: stacking an overlay onto its own primary would add the + // two periods together. + const primary = presentedSeries.find((s) => s.dataKey === m); + return { + dataKey: compareColumn(m), + label: `${headerLabel(m)} · ${compareLabel}`, + variant: 'comparison' as const, + ...(primary?.chartType ? { chartType: primary.chartType } : {}), + ...(primary?.yAxis ? { yAxis: primary.yAxis } : {}), + }; + }); const chartSeries = comparisonSeries.length > 0 - ? [...series.map((s) => ({ ...s, variant: (s as { variant?: string }).variant ?? 'current' })), ...comparisonSeries] - : series; + ? [...presentedSeries.map((s) => ({ ...s, variant: s.variant ?? 'current' })), ...comparisonSeries] + : presentedSeries; // Ordered-sequence charts (funnel/pyramid) need a DEFINED stage order. // `options.stageOrder` wins when the author states one explicitly; otherwise @@ -1186,10 +1402,10 @@ export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: // 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. + // criteria a key has to meet, for why `type`/`aria` are deliberately NOT + // here, and for where `xAxis`/`yAxis`/`series` go instead (#4229). 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 @@ -1217,7 +1433,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, ...chartPresentation, ...(effectiveCategoryOrder ? { categoryOrder: effectiveCategoryOrder } : {}) } as any} + schema={{ type: 'chart', chartType, data: chartData, xAxisKey, series: chartSeries, isAnimationActive: false, ...chartPresentation, ...authoredAxes, ...(effectiveCategoryOrder ? { categoryOrder: effectiveCategoryOrder } : {}) } as any} onChartClick={chartDrill} onSegmentClick={chartDrill} /> diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx index ca46623c7..02d5969c6 100644 --- a/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.chartConfig.test.tsx @@ -15,9 +15,16 @@ * `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`; + * authored one would shadow it: `type`, and the BINDING keys inside + * `xAxis`/`yAxis`/`series` (`ChartAxis.field`, `ChartSeries.name`); * - refused, because nothing on this path reads it: `aria`. * + * ⚠️ `xAxis`/`yAxis`/`series` were refused WHOLESALE until objectui#4229, which + * split them: their presentation half (per-series mark and axis binding, axis + * scale and chrome) is the author's and now merges onto the derived bindings — + * see `DatasetWidget.comboPresentation.test.tsx`. Only the data half is still + * refused, and it is pinned below. + * * 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. @@ -161,19 +168,23 @@ describe('DatasetWidget — chartConfig keys that ARE lowered (objectstack#7016) }); 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 () => { + // `xAxis` / `yAxis` / `series` were refused here too until objectui#4229, + // under the same criterion 2 — and that was half wrong. The DATA half of + // those keys is derived and still refused (below, and in + // `DatasetWidget.comboPresentation.test.tsx`); their PRESENTATION half is the + // author's and now merges forward, which is what makes an authored combo + // render as a combo. What remains refused, and is pinned here, is the part + // that would shadow the dataset's own derivation: the BINDINGS, i.e. the + // spec's `ChartAxis.field` and `ChartSeries.name`. + it('ignores an authored axis `field` 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 axes travel, stripped of the one key that names a column. + expect(lastChartSchema.xAxis).toEqual({ title: 'Authored X' }); + expect(lastChartSchema.yAxis).toEqual([{ min: 0, max: 5 }]); // The derived binding is untouched: the dimension is still the category axis. expect(lastChartSchema.xAxisKey).toBe('status'); }); @@ -181,7 +192,9 @@ describe('DatasetWidget — chartConfig keys that are REFUSED (objectstack#7016) 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. + // shape, one entry per selected measure) — not the authored array. An entry + // naming a measure outside the selection matches nothing, so its + // presentation is dropped with it: membership belongs to the dataset. expect(lastChartSchema.series).toHaveLength(1); expect(lastChartSchema.series[0].dataKey).toBe('total'); expect(lastChartSchema.series[0].name).toBeUndefined(); diff --git a/packages/plugin-dashboard/src/__tests__/DatasetWidget.comboPresentation.test.tsx b/packages/plugin-dashboard/src/__tests__/DatasetWidget.comboPresentation.test.tsx new file mode 100644 index 000000000..d685b4c32 --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/DatasetWidget.comboPresentation.test.tsx @@ -0,0 +1,323 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#4229 — an authored `combo` widget renders as a combo on the DATASET + * path, not as grouped bars. + * + * The QA run measured a widget authoring the spec's own combo shape + * (`series[].type` + `series[].yAxis` + two `yAxis` entries) and got + * **2 bars / 0 lines / 1 y-axis** where 1 bar + 1 line + 2 axes were authored. + * Two halves caused it, both in `DatasetWidget`: + * + * 1. `CHART_TYPE_MAP` had no `combo` entry, so a `combo` widget fell through + * the `?? 'bar'` default — bars, whatever the series said; + * 2. `chartConfigPresentation` refused to forward `series`/`xAxis`/`yAxis` at + * all, on the grounds that they are "derived from the dataset selection" — + * so the per-series mark and the left/right axis binding could never reach + * the renderer even once (1) was fixed. + * + * The ruling that settles who owns what: **the dataset owns DATA (series + * membership, the column each binding reads), the author owns PRESENTATION + * (mark, axis binding, scale, chrome), merged forward by name/key with the + * explicit binding winning.** That is objectui#2880's S2 rule — dual axes are + * `yAxis[].position` plus `series[].yAxis`, and a combo binds its axes by + * explicit binding first rather than by the implicit per-series-type guess — + * which PR #2883 landed in `ObjectChart` and which the dataset path never + * carried over. + * + * ## Where this stops, and why + * + * These assertions stop at the shape handed to the renderer, one step past the + * seam: they run the emitted schema through `normalizeChartSchema`, the ONE + * translation layer `ChartRenderer` puts between the schema and + * `AdvancedChartImpl` (#2880 S1), so what is pinned is what the renderer + * actually receives — not merely what this widget wrote down. + * + * They do NOT count recharts marks. That needs `ResponsiveContainer` mocked to + * a measured box, and `recharts` resolves inside `plugin-charts` alone (the + * same constraint `DatasetWidget.chartConfig.dom.test.tsx` records) — a + * `vi.mock('recharts')` in THIS package cannot even resolve the specifier. The + * mark half is already pinned there, against the exact shape asserted below: + * `AdvancedChartImpl.comboFromSeries.test.tsx` draws a `chartType: 'line'` + * series as a line and binds `yAxis: 'right'` to the right axis, and the combo + * branch renders both y-axes unconditionally. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +// The renderer's own translation layer, imported read-only so these assertions +// are made against what `AdvancedChartImpl` receives rather than a restatement +// of it. `plugin-charts` is a devDependency of this package and vitest aliases +// it to source. +import { normalizeChartSchema } from '@object-ui/plugin-charts/normalizeChartSchema'; + +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; +}); + +/** The card's fixture: a count and a percentage that must not share an axis. */ +const rows = [ + { assignee: 'ann', task_count: 12, avg_progress: 64 }, + { assignee: 'bob', task_count: 7, avg_progress: 88 }, +]; + +const fields = [ + { name: 'assignee', label: 'Assignee' }, + { name: 'task_count', label: 'Tasks' }, + { name: 'avg_progress', label: 'Avg progress' }, +]; + +/** `combo_count_vs_progress` as the QA run authored it. */ +const comboChartConfig = { + series: [ + { name: 'task_count', type: 'bar', yAxis: 'left' }, + { name: 'avg_progress', type: 'line', yAxis: 'right' }, + ], + yAxis: [ + { field: 'task_count', title: 'Tasks' }, + { field: 'avg_progress', title: 'Avg progress', position: 'right', min: 0, max: 100 }, + ], +}; + +const renderWidget = async (widget: Record) => { + const src = { queryDataset: vi.fn(async () => ({ rows, fields })) }; + render(); + await waitFor(() => expect(lastChartSchema).not.toBeNull()); +}; + +const comboWidget = (overrides: Record = {}) => ({ + type: 'combo', + dataset: 'tasks', + dimensions: ['assignee'], + values: ['task_count', 'avg_progress'], + chartConfig: comboChartConfig, + ...overrides, +}); + +/** + * What `AdvancedChartImpl` is handed for the two bindings this card is about. + * + * `series` is read verbatim: `ChartRenderer` forwards an array that already + * speaks the internal (`dataKey`) shape untouched, and this widget's does. + * `yAxes` is the normalized form of the authored `yAxis`. + */ +const asRenderer = (schema: any) => ({ + chartType: schema.chartType ?? normalizeChartSchema(schema).chartType, + series: schema.series, + yAxes: normalizeChartSchema(schema).yAxes, + xAxisKey: schema.xAxisKey ?? normalizeChartSchema(schema).xAxisKey, +}); + +describe('DatasetWidget — an authored combo reaches the renderer as a combo (#4229)', () => { + // Half 1. `combo` is a `ChartTypeSchema` member since spec 17.0.0-rc.1 and the + // renderer draws it distinctly, so it maps to itself instead of falling + // through the `?? 'bar'` default that produced the measured bars. + it('resolves the `combo` family instead of falling through to bar', async () => { + await renderWidget(comboWidget()); + expect(asRenderer(lastChartSchema).chartType).toBe('combo'); + }); + + // Half 2, the mark: `series[].type` is presentation and merges onto the + // derived binding. Without it the line measure drew as the second bar. + it('merges the authored per-series mark onto the derived series', async () => { + await renderWidget(comboWidget()); + const { series } = asRenderer(lastChartSchema); + expect(series.map((s: any) => [s.dataKey, s.chartType])).toEqual([ + ['task_count', 'bar'], + ['avg_progress', 'line'], + ]); + // The label still comes from the dataset field, not from the author's entry + // (which declared none) — membership and its labelling stay derived. + expect(series.map((s: any) => s.label)).toEqual(['Tasks', 'Avg progress']); + }); + + // Half 2, the axes: two declared entries are what turn on the secondary axis, + // and `series[].yAxis` is what binds a measure to it. The percentage measure + // must not be plotted against the count's scale. + it('declares two y-axes and binds each series to the authored one', async () => { + await renderWidget(comboWidget()); + const { series, yAxes } = asRenderer(lastChartSchema); + expect(series.map((s: any) => s.yAxis)).toEqual(['left', 'right']); + expect(yAxes).toHaveLength(2); + expect(yAxes?.[1].position).toBe('right'); + // The axis presentation travels with it: the percentage axis is pinned to + // 0–100 rather than sharing the count's auto domain. + expect(yAxes?.[1].min).toBe(0); + expect(yAxes?.[1].max).toBe(100); + expect(yAxes?.map((a: any) => a.title)).toEqual(['Tasks', 'Avg progress']); + }); + + // The membership pin. An authored entry naming a measure the dataset did not + // select is IGNORED — it cannot add a series, and it cannot re-point one. + it('ignores an authored series naming a measure outside the selection', async () => { + await renderWidget( + comboWidget({ + chartConfig: { + series: [ + { name: 'avg_progress', type: 'line', yAxis: 'right' }, + { name: 'not_a_measure', type: 'line', yAxis: 'right', color: '#ff0000' }, + ], + }, + }), + ); + const { series } = asRenderer(lastChartSchema); + expect(series.map((s: any) => s.dataKey)).toEqual(['task_count', 'avg_progress']); + expect(series.some((s: any) => s.color === '#ff0000')).toBe(false); + }); + + // The control on the other side of the match: a derived series the author + // said nothing about carries no presentation at all, so the renderer's family + // default decides its mark — `chartType` is absent, not defaulted here. + it('leaves a derived series with no authored entry untouched', async () => { + await renderWidget( + comboWidget({ chartConfig: { series: [{ name: 'avg_progress', type: 'line', yAxis: 'right' }] } }), + ); + const { series } = asRenderer(lastChartSchema); + expect(series[0]).toEqual({ dataKey: 'task_count', label: 'Tasks' }); + expect(series[1]).toMatchObject({ dataKey: 'avg_progress', chartType: 'line', yAxis: 'right' }); + }); + + // `ChartAxis.field` is the one DATA key on an axis — it names the plotted + // column — so it does not travel, and the derived category binding is what + // the renderer keeps. This is what makes forwarding the axes safe: with + // `field` gone there is no channel by which an authored axis could name a + // series (`normalizeChartSchema` synthesises series from `yAxis[].field`). + it('does not let an authored axis `field` reach the renderer', async () => { + await renderWidget( + comboWidget({ + chartConfig: { + xAxis: { field: 'not_a_column', title: 'By assignee' }, + yAxis: [{ field: 'not_a_measure', title: 'Tasks' }], + }, + }), + ); + const schema = lastChartSchema; + expect(schema.xAxis).toEqual({ title: 'By assignee' }); + expect(schema.yAxis).toEqual([{ title: 'Tasks' }]); + // The derived bindings are untouched. + expect(asRenderer(schema).xAxisKey).toBe('assignee'); + expect(asRenderer(schema).series.map((s: any) => s.dataKey)).toEqual(['task_count', 'avg_progress']); + }); + + // A second `yAxis` entry that carries nothing but its own existence still + // declares the secondary axis — the COUNT is presentation, and stripping + // `field` must not collapse it (`hasDualAxis` is `yAxes.length > 1`). + it('keeps an axis slot that declared only its `field`', async () => { + await renderWidget( + comboWidget({ chartConfig: { yAxis: [{ field: 'task_count' }, { field: 'avg_progress' }] } }), + ); + expect(asRenderer(lastChartSchema).yAxes).toHaveLength(2); + }); + + // A `type` the renderer cannot compose on one cartesian plot is dropped + // rather than passed through: as `chartType` it would count as a family + // disagreement, flip the chart into a combo, and then draw as a bar anyway. + it('drops a per-series type that is not bar/line/area', async () => { + await renderWidget( + comboWidget({ chartConfig: { series: [{ name: 'avg_progress', type: 'pie' }] } }), + ); + expect('chartType' in asRenderer(lastChartSchema).series[1]).toBe(false); + }); +}); + +describe('DatasetWidget — the merge is presentation-only (#4229 controls)', () => { + // The guard against the merge forwarding stale keys the old code stripped: + // a widget that authors no chartConfig emits exactly what it emitted before. + it('emits no axis keys and a byte-identical derived series when nothing is authored', async () => { + await renderWidget({ + type: 'bar', + dataset: 'tasks', + dimensions: ['assignee'], + values: ['task_count', 'avg_progress'], + }); + expect('xAxis' in lastChartSchema).toBe(false); + expect('yAxis' in lastChartSchema).toBe(false); + expect(lastChartSchema.chartType).toBe('bar'); + expect(lastChartSchema.series).toEqual([ + { dataKey: 'task_count', label: 'Tasks' }, + { dataKey: 'avg_progress', label: 'Avg progress' }, + ]); + }); + + // The same for a chartConfig that declares only chrome: the chrome lowers + // (objectstack#7016) and the bindings stay derived. + it('leaves the bindings alone for a chartConfig that declares only chrome', async () => { + await renderWidget({ + type: 'line', + dataset: 'tasks', + dimensions: ['assignee'], + values: ['task_count'], + chartConfig: { title: 'Tasks by assignee', showLegend: false }, + }); + expect(lastChartSchema.title).toBe('Tasks by assignee'); + expect(lastChartSchema.showLegend).toBe(false); + expect('xAxis' in lastChartSchema).toBe(false); + expect('yAxis' in lastChartSchema).toBe(false); + expect(lastChartSchema.series).toEqual([{ dataKey: 'task_count', label: 'Tasks' }]); + }); + + // A non-combo family gains the same merge — `series[].type` is how the spec + // says "this one measure is a line", and #2945 already taught the renderer to + // DERIVE a combo from that disagreement. The dataset path now delivers it. + it('lets a bar widget declare one line series (a derived combo)', async () => { + await renderWidget({ + type: 'bar', + dataset: 'tasks', + dimensions: ['assignee'], + values: ['task_count', 'avg_progress'], + chartConfig: { series: [{ name: 'avg_progress', type: 'line', yAxis: 'right' }] }, + }); + const { chartType, series } = asRenderer(lastChartSchema); + // The widget's own family is unchanged — the renderer derives the combo. + expect(chartType).toBe('bar'); + expect(series[1]).toMatchObject({ chartType: 'line', yAxis: 'right' }); + }); +}); + +describe('DatasetWidget — a comparison overlay follows its own measure (#4229)', () => { + // `compareTo` adds one overlay series per compared measure. It is the SAME + // measure a period back, so it has to take that measure's merged mark and + // axis; left to the renderer's positional guess, the overlay of a bar/left + // measure drew as a line on the right axis the moment the chart became a + // combo. + it('gives each overlay the mark and axis of the series it overlays', async () => { + const src = { + queryDataset: vi.fn(async () => ({ + rows: rows.map((r) => ({ ...r, task_count__compare: 9, avg_progress__compare: 55 })), + fields, + })), + }; + render( + , + ); + await waitFor(() => expect(lastChartSchema).not.toBeNull()); + + const series = lastChartSchema.series; + expect(series.map((s: any) => s.dataKey)).toEqual([ + 'task_count', 'avg_progress', 'task_count__compare', 'avg_progress__compare', + ]); + expect(series.slice(2).map((s: any) => [s.chartType, s.yAxis, s.variant])).toEqual([ + ['bar', 'left', 'comparison'], + ['line', 'right', 'comparison'], + ]); + // Stacking is NOT inherited: an overlay stacked onto its own primary would + // add the two periods together. + expect(series.every((s: any) => s.stack === undefined)).toBe(true); + }); +});