diff --git a/.changeset/report-chart-authored-chrome.md b/.changeset/report-chart-authored-chrome.md new file mode 100644 index 0000000000..d7e2fe5ee0 --- /dev/null +++ b/.changeset/report-chart-authored-chrome.md @@ -0,0 +1,44 @@ +--- +'@object-ui/plugin-report': patch +'@object-ui/plugin-dashboard': patch +'@object-ui/core': patch +--- + +fix(plugin-report): forward the chart chrome and series presentation `ReportChartSchema` declares (objectui#4877) + +A report's embedded chart forwarded exactly six keys to the registered chart +component — `chartType`, `data`, `height`, `isAnimationActive`, `series`, +`xAxisKey`. Everything else `ReportChartSchema` declares as authorable never +left the report renderer, so it was inert metadata: the author writes it, the +schema accepts it, nothing reads it. + +`showLegend` was the sharpest case because dropping it does not merely ignore +the author, it INVERTS them: `AdvancedChartImpl` computes +`legendVisible = showLegend !== false`, so an absent value means the legend is +on and an explicit `showLegend: false` still drew one. + +Now lowered, under objectui#4229's ruled data/presentation split: + +- chrome — `showLegend`, `showDataLabels`, `colors` (both the positional-palette + array and the per-category record), `subtitle`, `description`, `annotations`, + `interaction`, `height`; +- per-series presentation — `color`, `stack`, `type`, `yAxis`, `dashArray`, + `opacity`, `variant`, matched by `series[].name` so series MEMBERSHIP stays + with the dataset. + +`title` is deliberately not forwarded: the report renderer paints it as its own +heading above the plot, and forwarding it would draw a second one inside the +chart's frame. `aria` is not lowered either — nothing on this path reads it +(`AdvancedChartImpl` has no `aria` prop, and this renderer hands the component a +schema directly rather than through `SchemaRenderer`'s flat ARIA injection), so +forwarding it would move declared-but-unread one layer down. + +The two helpers (`chartConfigPresentation`, `mergeAuthoredPresentation`) moved +from `plugin-dashboard`'s `DatasetWidget` to `@object-ui/core` beside +`buildChartSeries`, the derivation they merge onto, so both surfaces lower one +vocabulary once instead of keeping a second copy (the duplication objectui#4389 +filed as a defect). `@object-ui/core` additionally exports `mergeAuthoredSeries` +— the series merge alone — for a surface whose axes are bare dimension/measure +NAME strings rather than spec `ChartAxis` objects, which is what a report chart +declares. `DatasetWidget` re-exports both names, so its public surface and its +rendering are unchanged. diff --git a/.changeset/report-chart-null-category-bucket.md b/.changeset/report-chart-null-category-bucket.md new file mode 100644 index 0000000000..7047b2095d --- /dev/null +++ b/.changeset/report-chart-null-category-bucket.md @@ -0,0 +1,31 @@ +--- +'@object-ui/plugin-report': patch +--- + +fix(plugin-report): route a report's embedded chart through `buildChartSeries` so a NULL category is bucketed (objectui#4878) + +`DatasetReportChart` built its rows as `relabelDimensions(state.rows, …)` and +handed them to the registered chart component verbatim. Nothing on that path +bucketed a null dimension value, so a report chart passed the renderer a null +category — the exact input objectui#4466 measured as drawing **no mark at all**. +The cost is not an empty chart but a quietly wrong one: the null group vanishes +while the y-axis scale still accommodates it, so the chart reads as valid data. + +The dashboard and chart-view surfaces never had the defect because they route +through `buildChartSeries` (`@object-ui/core`), where the whole null-category +family was fixed. The report chart now routes through it too, so those +properties are INHERITED rather than re-derived on a third surface: + +- the null bucket itself (objectui#4466); +- its label read from the locale bundle at the call site — `@object-ui/core` is + React-free, so a zh console would otherwise draw the bar and label it `(None)` + (objectui#4500); +- bucket IDENTITY separate from the bucket label, so a stored value that + literally spells `(None)` stays a different group (objectui#4508). + +objectui#4020's three-level measure display name still outranks the label the +derivation assigns, including for an `{ en, 'zh-CN' }` label record: core holds +no i18n provider and picks first-string-wins, which is exactly the defect class +#4020 closed. + +A report whose chart has no null group is unchanged, byte for byte. diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 91506411d1..7f52745fd3 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -59,6 +59,10 @@ export * from './utils/dashboard-filters.js'; export * from './utils/merge-filters.js'; export * from './utils/compare-to.js'; export * from './utils/chart-series.js'; +// The AUTHORED half of a dataset-bound chart (objectui#4229's data/presentation +// split), shared by the dashboard widget and the report's embedded chart so the +// same spec keys are lowered identically on both (objectui#4877). +export * from './utils/chart-presentation.js'; // The ONE number-display formatter (objectui#4033) — grouping policy, display // locale and the percent convention. It lived in `@object-ui/i18n` until // objectui#4576; it is pure, and living above `core` was what kept diff --git a/packages/core/src/utils/chart-presentation.ts b/packages/core/src/utils/chart-presentation.ts new file mode 100644 index 0000000000..42a9c26707 --- /dev/null +++ b/packages/core/src/utils/chart-presentation.ts @@ -0,0 +1,348 @@ +/** + * 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. + */ + +/** + * chart-presentation — the AUTHORED half of a dataset-bound chart, lowered onto + * the bindings {@link buildChartSeries} derived from the dataset selection. + * + * `chart-series.ts` beside this file owns the DATA half: which columns become + * series, which rows, which buckets. This file owns everything the author gets + * to say about how that data LOOKS — the ruled data/presentation split of + * objectui#4229, stated once: + * + * - **Data (derived, never forwarded)** — series MEMBERSHIP and the column each + * binding reads. Concretely `buildChartSeries`' `dataKey`s, `xAxisKey`, and + * the spec's two binding keys `ChartSeries.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`, + * the axis definitions' `title`/`format`/`min`/`max`/`stepSize`/ + * `showGridLines`/`position`/`logarithmic`, and the chart chrome + * (`showLegend`, `showDataLabels`, `colors`, `annotations`, …). + * + * ## Why it lives in `@object-ui/core` rather than in one plugin + * + * It was written for `plugin-dashboard`'s `DatasetWidget` (#3135 → objectstack#7016 + * → #4229) and lifted here by objectui#4877, when `plugin-report`'s embedded + * report chart turned out to need the SAME merge over the same spec shapes: the + * report was forwarding six keys and dropping every authored chrome key the + * schema declares, and re-deriving this beside the dashboard's copy is exactly + * the duplication objectui#4389 filed as a defect (two longhand copies of the + * analytics label net, one per plugin, drifting apart). + * + * Everything here is a pure data transform over plain records, so it sits below + * both plugins with no React and no i18n — the same layering `chart-series.ts` + * already has (see {@link OptionLabelTranslator} there for why an i18n-resolved + * string always arrives as an ARGUMENT rather than being read here). + * + * ## Two entry points, because the two surfaces declare axes differently + * + * A dashboard widget's `chartConfig` spells its axes as spec `ChartAxis` + * OBJECTS. A report's `chart.xAxis` / `chart.yAxis` are bare dimension/measure + * NAME strings — on that surface the axes are pure DATA (they ARE the selection) + * and carry no presentation at all. So: + * + * - {@link mergeAuthoredPresentation} is the object-axis entry point: series + * merge + axis presentation, for a caller whose axes are `ChartAxis` objects. + * - {@link mergeAuthoredSeries} is the series merge ALONE, for a caller whose + * axes are names. That split is structural, not a guard: handing a bare + * string to {@link mergeAuthoredPresentation} would run it through + * `axisPresentation`, which reads nothing off a string and would synthesise a + * `yAxis: [{}]` entry — a y-axis declaring nothing but its own existence, + * which is precisely how the COUNT of entries turns on a secondary axis. A + * surface that cannot author axis presentation should not be able to reach + * the code that reads it. + */ + +import type { ChartSeriesBinding } from './chart-series'; + +/** 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. + * + * **First-string-wins, deliberately, and deliberately NOT locale-aware** — this + * package is React-free and holds no i18n provider, so it cannot know which + * limb a console wants. A caller that CAN resolve the language (the report + * renderer, via `pickLocalized`) resolves the label itself and overrides the + * merged one; see objectui#4020, whose whole point is that picking the first + * limb paints English on a zh console. + */ +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. + */ +export 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. + */ +export 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 an authored `series[]` array's PRESENTATION onto the bindings the + * dataset selection derived — the series half of #4229's split, and the whole + * of it for a surface whose axes are names rather than objects (see the file + * header). + * + * 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 the chart config; + * - a derived series with no authored entry keeps the family default, so every + * surface that never wrote `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). + * + * The FIRST entry naming a measure wins; a later duplicate cannot silently + * reconfigure a series the author already described. + * + * @param derived the bindings {@link buildChartSeries} produced from the selection + * @param authoredSeries the authored `series` array (anything, incl. absent) + */ +export function mergeAuthoredSeries( + derived: ChartSeriesBinding[], + authoredSeries: unknown, +): MergedChartSeries[] { + const authored = new Map>(); + for (const entry of Array.isArray(authoredSeries) ? authoredSeries : []) { + if (!isRecord(entry)) continue; + const name = typeof entry.name === 'string' ? entry.name : undefined; + if (name && !authored.has(name)) authored.set(name, entry); + } + return derived.map((s) => { + const entry = authored.get(s.dataKey); + return entry ? { ...s, ...seriesPresentation(entry) } : s; + }); +} + +/** + * Merge an authored chart config's PRESENTATION onto the series and axes the + * dataset selection derived — the one place that happens for a surface whose + * axes are spec `ChartAxis` OBJECTS (#4229). + * + * The series half is {@link mergeAuthoredSeries}; see it for the match rule and + * for why membership is safe. The axes half reads `xAxis` / `yAxis` through + * {@link axisPresentation}, which is why a surface spelling those as bare NAME + * strings must call `mergeAuthoredSeries` directly instead (file header). + * + * @param derived the bindings {@link buildChartSeries} produced from the selection + * @param raw the authored chart config (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 series = mergeAuthoredSeries(derived, config.series); + + 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 }; +} + +/** + * Lower an authored chart config's CHROME (spec `ChartConfigSchema` / + * `ReportChartSchema` — the same keys on both) onto the chart schema a + * dataset-bound surface hands the renderer. + * + * ## Why this is a whitelist and not a spread + * + * Until #3135 NONE of a dashboard widget's `chartConfig` reached the renderer: + * the 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 lowered 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.** `{ 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 + * exists to remove. + * 2. **It does not fight the dataset derivation.** `type` stays out: the + * calling surface's own type already picks the chart family, which is the + * dataset path's chart-family channel. + * + * `aria` is the one declared key with **no reader at all**: `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) rather than papered over + * with a caller-side flattening that would also collide with the accessible + * name `description` already sets — reported back to objectstack#5175's + * narrowing half instead. + * + * `title` IS emitted here. A caller that paints the title itself (the report + * renderer's own `h3` above the chart) must drop it from the result, or the + * chart draws a second one. + * + * @param raw the authored chart config (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 = isRecord(raw) ? raw : {}; + 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; +} diff --git a/packages/plugin-dashboard/src/DatasetWidget.tsx b/packages/plugin-dashboard/src/DatasetWidget.tsx index 1448767717..9b612f3ea0 100644 --- a/packages/plugin-dashboard/src/DatasetWidget.tsx +++ b/packages/plugin-dashboard/src/DatasetWidget.tsx @@ -58,8 +58,12 @@ import { pivotDimensionValue, pivotCellKey, compareToTrendLabelKey, + // The authored half of the same split — moved to core beside `buildChartSeries` + // so this widget and the report's embedded chart lower one vocabulary once + // (objectui#4877). Re-exported below under their original names. + chartConfigPresentation, + mergeAuthoredPresentation, type ChartSegmentClickEvent, - type ChartSeriesBinding, type CompareToConfig, type DatasetResultField, type DatasetDrillRange, @@ -369,289 +373,27 @@ const CHART_TYPE_MAP: Record = { }; /** - * 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. + * The authored chart CHROME and the series/axis presentation merge, both of + * which now live in `@object-ui/core`'s `chart-presentation` beside + * `buildChartSeries` — the derivation they are merged onto (objectui#4877). * - * ## Why this is a whitelist and not a spread + * They were written here (#3135 → objectstack#7016 → #4229) and moved when the + * report renderer's embedded chart turned out to need the SAME merge over the + * same spec keys: `ReportChartSchema` and `ChartConfigSchema` declare one + * vocabulary, and a second copy of the split beside this one is precisely the + * duplication objectui#4389 filed as a defect. The doctrine — the two + * admission criteria, the data/presentation ruling, why `aria` stays + * unforwarded — travelled with the code; see that module's header. * - * 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.** `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 - * 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; -} - -/** 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. + * Re-exported under their original names so this module's public surface is + * unchanged. */ -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 { + chartConfigPresentation, + mergeAuthoredPresentation, + type AuthoredSeriesPresentation, + type MergedChartSeries, +} from '@object-ui/core'; export function DatasetWidget({ widget, dataSource }: { widget: any; dataSource: unknown }) { const datasetName = String(widget?.dataset ?? ''); diff --git a/packages/plugin-report/src/DatasetReportRenderer.tsx b/packages/plugin-report/src/DatasetReportRenderer.tsx index ce548e0dfa..087b3ef059 100644 --- a/packages/plugin-report/src/DatasetReportRenderer.tsx +++ b/packages/plugin-report/src/DatasetReportRenderer.tsx @@ -65,6 +65,17 @@ import { buildDatasetFieldHelpers, buildDatasetDrillFilter, relabelDimensions, + // The dataset→chart derivation the dashboard and the chart view have always + // used, adopted here by objectui#4878: it is where the whole null-category + // family lives (#4466 / #4497 / #4673 / #4500 / #4508), so routing through it + // INHERITS those fixes instead of re-deriving them on a third surface. + buildChartSeries, + // The authored half of objectui#4229's data/presentation split (objectui#4877). + // `mergeAuthoredSeries` — not `mergeAuthoredPresentation` — because a report's + // `chart.xAxis`/`chart.yAxis` are bare dimension/measure NAME strings, i.e. + // pure data on this surface; see the call site. + chartConfigPresentation, + mergeAuthoredSeries, pivotBucketId, pivotDimensionValue, pivotCellKey, @@ -683,6 +694,24 @@ function authoredSeriesLabel(series: unknown, measure: string, language: string * chart plugin isn't loaded, or the chart is incomplete, we render nothing and * let the grouped table stand alone. Before this, a dataset-bound report's * `chart` config was authorable in Studio but never rendered anywhere. + * + * ## What reaches the chart component (objectui#4877 / objectui#4878) + * + * This slot forwarded exactly six keys — `chartType`, `data`, `height`, + * `isAnimationActive`, `series`, `xAxisKey` — which made it the one dataset + * chart surface that shared neither half of the ruled split: + * + * - the DATA half now routes through `buildChartSeries` (objectui#4878), so the + * null-category family it owns is inherited rather than re-derived; the rows + * used to reach the renderer with a raw null category, which draws no mark; + * - the PRESENTATION half now routes through `chartConfigPresentation` + + * `mergeAuthoredSeries` (objectui#4877), so the chrome and per-series keys + * `ReportChartSchema` declares stop being inert metadata. `showLegend: false` + * was the sharpest of those: dropped, it read as absent, and absent means the + * legend is ON — the author's explicit value inverted in effect. + * + * Both helpers live in `@object-ui/core` beside each other, which is what keeps + * this surface and the dashboard widget lowering ONE vocabulary once. */ function DatasetReportChart({ dataset, @@ -723,6 +752,13 @@ function DatasetReportChart({ ); const ChartComponent = useRegistryComponent('chart'); const { fieldLabel } = useSafeFieldLabel(); + // objectui#4878 — the null-category bucket's LABEL. `@object-ui/core` is + // React-free and cannot read the locale bundle, so `buildChartSeries` falls + // back to the English `NULL_CATEGORY_LABEL`; the resolved string has to come + // from HERE, the layer that holds the provider (objectui#4500 made exactly + // this division on the dashboard, and ObjectChart makes it too). Without it a + // zh console would draw the bar — and label it `(None)`. + const tt = useSafeTranslate(); // objectui#4575 — the single-value metric below is a MEASURE like any other // and follows the display locale. (The series charts render their own labels // through the chart component, not through `formatMeasure`.) @@ -812,6 +848,72 @@ function DatasetReportChart({ // (the grouped table beneath still shows the exact numbers). if (!ChartComponent) return null; + // ── The DATA half: derived from the selection, never authored (#4229) ───── + // + // objectui#4878 — the rows and the series binding come from the SHARED + // derivation every other dataset chart surface uses. This path used to hand + // `relabelDimensions(state.rows, …)` straight to the renderer, which is the + // pre-#4466 answer: a null dimension value reached the chart raw and drew NO + // MARK, so the report silently understated its own data (#4466 measured the + // dominant group vanishing while the y-axis still accommodated it). Every + // property of that family — the bucket itself (#4466), its localized label + // (#4500), and the identity that keeps a stored `'(None)'` apart from the + // null group (#4508) — is a property of `buildChartSeries`, so it arrives + // here by inheritance rather than by a third re-derivation. + // + // The selection is exactly one dimension × one measure, so this takes the + // helper's single-dimension branch and returns ONE series; the pivot branch + // (2+ dimensions) is unreachable from a report chart, whose schema declares a + // single `xAxis`/`yAxis` pair. + const { data: chartData, xAxisKey, series: derivedSeries } = buildChartSeries( + relabelDimensions(state.rows, dimensionLabels), + [xAxis], + [yAxis], + state.fields, + { nullCategoryLabel: tt('chart.nullCategory', '(None)') }, + ); + + // ── The PRESENTATION half: authored, merged forward (#4229) ────────────── + // + // objectui#4877 — `ReportChartSchema` declares per-series `color`/`stack`/ + // `type`/`yAxis`/`dashArray`/`opacity`/`variant`, and this path forwarded + // none of them. `mergeAuthoredSeries` is the same merge the dashboard makes + // over the same spec shape, matched by `series[].name` → derived `dataKey`, + // so membership stays with the dataset: an entry naming a measure this chart + // does not plot is ignored. + // + // Deliberately NOT `mergeAuthoredPresentation`: that entry point also reads + // `xAxis`/`yAxis` as spec `ChartAxis` OBJECTS, and on THIS surface they are + // bare dimension/measure name strings — the selection itself. Feeding them to + // it would return `axes.yAxis = [{}]`, one empty entry, and the COUNT of + // y-axis entries is what declares a secondary axis. The series-only entry + // point makes that unreachable by construction rather than by a guard. + const authoredSeries = mergeAuthoredSeries(derivedSeries, chart.series); + // #4020's three-level display name OUTRANKS both of the labels above: the + // derivation's `fields[].label` (level ② without the i18n field-label + // convention `headerLabel` applies) and `mergeAuthoredSeries`' own pick, + // which resolves an `{ en, 'zh-CN' }` label first-string-wins because core + // holds no provider — the very thing that would paint English on a zh + // console. `measureLabel` already resolved ① through `pickLocalized` and ② + // through `headerLabel`, so it wins outright. + const chartSeries = authoredSeries.map((s) => + s.dataKey === yAxis ? { ...s, label: measureLabel } : s, + ); + + // The authored CHROME — `showLegend`, `showDataLabels`, `colors`, `subtitle`, + // `description`, `annotations`, `interaction`, `height` — lowered by the same + // whitelist the dashboard uses (objectui#4877). `showLegend: false` was not + // merely dropped before but INVERTED in effect: `AdvancedChartImpl` computes + // `legendVisible = showLegend !== false`, so an absent value means the legend + // is on and the author's explicit `false` drew one anyway. + // + // `title` is dropped from the result on purpose: this renderer paints the + // report chart's title itself, as the `h3` below, and forwarding it as well + // would draw a SECOND one inside the chart's own frame. `aria` is not lowered + // by the whitelist at all — nothing on this path reads it (see that helper's + // header for the ruling and where it is tracked). + const { title: _chartOwnTitle, ...chrome } = chartConfigPresentation(chart); + return (
{title ?

{title}

: null} @@ -819,19 +921,23 @@ function DatasetReportChart({
diff --git a/packages/plugin-report/src/__tests__/DatasetReportRenderer.chartChrome.test.tsx b/packages/plugin-report/src/__tests__/DatasetReportRenderer.chartChrome.test.tsx new file mode 100644 index 0000000000..ca6b6bcc21 --- /dev/null +++ b/packages/plugin-report/src/__tests__/DatasetReportRenderer.chartChrome.test.tsx @@ -0,0 +1,287 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#4877 — a report's embedded chart dropped every authored chrome key. + * + * `DatasetReportChart` forwarded exactly six keys to the registered chart + * component — `chartType`, `data`, `height`, `isAnimationActive`, `series`, + * `xAxisKey`. Everything else `ReportChartSchema` declares as authorable was + * inert metadata: the author writes it, the schema accepts it, nothing reads + * it. `showLegend` was the sharpest case because it is not merely ignored but + * INVERTED in effect — `AdvancedChartImpl` computes + * `legendVisible = showLegend !== false`, so an absent value means the legend is + * ON and an author's explicit `false` drew one anyway. + * + * The fix lowers the same whitelist the dashboard already used + * (`chartConfigPresentation`) plus the series half of #4229's split + * (`mergeAuthoredSeries`), both moved to `@object-ui/core` so the two surfaces + * lower ONE vocabulary once. + * + * DIRECTIONS, written before the reverse verification was run. The mutation is + * to stop forwarding — drop the `...chrome` spread and hand + * `series: [{ dataKey: yAxis, label: measureLabel }]` again: + * + * Measured: 11 red / 8 green, matching the prediction case for case. + * + * - every "reaches the chart component" case goes RED, and the `showLegend` + * one goes red in the card's own way: the key is `undefined`, i.e. absent, + * i.e. a legend; + * - the TITLE cases stay GREEN on both sides. `title` is deliberately NOT + * forwarded — this renderer paints it as its own `h3` — so those pin a + * decision, not a change, and would go red only if a future edit spread the + * chart config wholesale and drew the title twice; + * - the `aria` case likewise stays GREEN on both sides: it is the one declared + * key with no reader anywhere on this path, so it is deliberately not + * lowered and the assertion records that as a decision. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ComponentRegistry } from '@object-ui/core'; +import { DatasetReportRenderer } from '../DatasetReportRenderer'; + +let captured: { schema: Record } | null = null; + +beforeEach(() => { + captured = null; + ComponentRegistry.register('chart', (props: any) => { + captured = props; + return null; + }); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +const RESULT = { + rows: [ + { stage: 'Qualification', amount: 120 }, + { stage: 'Negotiation', amount: 80 }, + ], + fields: [ + { name: 'stage', type: 'text', label: 'Stage' }, + { name: 'amount', type: 'number', label: 'Amount' }, + ], +}; + +const sourceOf = (result: unknown) => ({ queryDataset: vi.fn(async () => result) }); + +const BASE = { + name: 'pipeline_by_stage', + type: 'tabular', + dataset: 'pipeline_metrics', + rows: ['stage'], + values: ['amount'], +}; + +/** The schema the registered chart component was handed for this `chart` block. */ +async function chartSchema(chart: Record) { + render( + , + ); + await waitFor(() => expect(captured?.schema?.data?.length).toBe(2)); + return captured!.schema; +} + +const CHART = { type: 'bar', xAxis: 'stage', yAxis: 'amount' } as const; + +describe('report chart chrome — the declared-but-unread set now reaches the renderer (objectui#4877)', () => { + it('`showLegend: false` arrives as `false`, not as absent', async () => { + // The card's headline. Absent is not a neutral value here: the renderer + // reads `showLegend !== false`, so dropping the key turned the author's + // "no legend" into "legend". + const schema = await chartSchema({ ...CHART, showLegend: false }); + expect(schema.showLegend).toBe(false); + }); + + it('`showLegend: true` also travels — the author said it, not the default', async () => { + const schema = await chartSchema({ ...CHART, showLegend: true }); + expect(schema.showLegend).toBe(true); + }); + + it('`showDataLabels` travels', async () => { + const schema = await chartSchema({ ...CHART, showDataLabels: true }); + expect(schema.showDataLabels).toBe(true); + }); + + it('a `colors` ARRAY travels as the positional palette', async () => { + const schema = await chartSchema({ ...CHART, colors: ['#f00', '#0f0'] }); + expect(schema.colors).toEqual(['#f00', '#0f0']); + // The two arms reach the renderer through two DIFFERENT props; an array is + // not a per-category map and must not become one. + expect(schema.categoryColors).toBeUndefined(); + }); + + it('a `colors` RECORD travels as the per-category map', async () => { + // `ReportChartSchema.colors` is `string[] | Record`, the + // same overload the dashboard's split already rules on. + const schema = await chartSchema({ ...CHART, colors: { Qualification: '#f00' } }); + expect(schema.categoryColors).toEqual({ Qualification: '#f00' }); + expect(schema.colors).toBeUndefined(); + }); + + it('`subtitle` and `description` travel', async () => { + const schema = await chartSchema({ + ...CHART, + subtitle: 'Open pipeline only', + description: 'Amount by stage for the current quarter', + }); + expect(schema.subtitle).toBe('Open pipeline only'); + expect(schema.description).toBe('Amount by stage for the current quarter'); + }); + + it('`annotations` and `interaction` travel', async () => { + const annotations = [{ type: 'line', axis: 'y', value: 100, label: 'Target' }]; + const schema = await chartSchema({ ...CHART, annotations, interaction: { brush: true } }); + expect(schema.annotations).toEqual(annotations); + expect(schema.interaction).toEqual({ brush: true }); + }); + + it('an authored `height` wins over the renderer default', async () => { + const schema = await chartSchema({ ...CHART, height: 420 }); + expect(schema.height).toBe(420); + }); + + it('a non-positive `height` falls back to the default instead of collapsing the plot', async () => { + // The whitelist drops it, which leaves the 280 default standing — an + // invisible plot is a worse answer than a default-sized one. + const schema = await chartSchema({ ...CHART, height: 0 }); + expect(schema.height).toBe(280); + }); + + it('a chart authoring NO chrome keeps the renderer defaults', async () => { + // Byte-for-byte the pre-change forwarding for the overwhelming majority of + // stored reports: nothing authored, nothing added. + const schema = await chartSchema({ ...CHART }); + for (const key of [ + 'showLegend', + 'showDataLabels', + 'colors', + 'categoryColors', + 'subtitle', + 'description', + 'annotations', + 'interaction', + ]) { + expect(schema[key]).toBeUndefined(); + } + expect(schema.height).toBe(280); + }); +}); + +describe('report chart chrome — `title` stays the report renderer’s own (decision, not change)', () => { + it('the chart component is NOT handed a title', async () => { + // Green before and after. This renderer paints the title as an `h3` above + // the plot; forwarding it as well would draw a SECOND one inside the + // chart's own frame. + const schema = await chartSchema({ ...CHART, title: 'Pipeline by stage' }); + expect(schema.title).toBeUndefined(); + }); + + it('and the report still paints it itself', async () => { + render( + , + ); + const heading = await screen.findByRole('heading', { name: 'Pipeline by stage' }); + expect(heading).toBeInTheDocument(); + }); +}); + +describe('report chart chrome — `aria` is deliberately not lowered', () => { + it('does not reach the chart component under any spelling', async () => { + // Green before and after, and recorded on purpose: `aria` is the one key + // `ReportChartSchema` declares that NOTHING on this path reads — + // `AdvancedChartImpl` has no `aria` prop, and this renderer hands the + // component a schema directly rather than through `SchemaRenderer`'s flat + // ARIA injection. Forwarding it would move declared-but-unread one layer + // down, which is the very failure this card removes. + const schema = await chartSchema({ ...CHART, aria: { ariaLabel: 'Pipeline chart' } }); + expect(schema.aria).toBeUndefined(); + expect(schema.ariaLabel).toBeUndefined(); + }); +}); + +describe('report chart series presentation — the #4229 split, series half (objectui#4877)', () => { + it('every authored presentation key survives onto the plotted series', async () => { + const schema = await chartSchema({ + ...CHART, + series: [ + { + name: 'amount', + type: 'line', + color: '#f00', + stack: 'a', + yAxis: 'right', + dashArray: '4 2', + opacity: 0.5, + variant: 'comparison', + }, + ], + }); + expect(schema.series[0]).toMatchObject({ + dataKey: 'amount', + chartType: 'line', + color: '#f00', + stack: 'a', + yAxis: 'right', + dashArray: '4 2', + opacity: 0.5, + variant: 'comparison', + }); + }); + + it('MEMBERSHIP stays with the dataset — an entry naming another measure is ignored', async () => { + // An author cannot add, remove or re-point a series from the chart block; + // `values` / `yAxis` decide what is plotted. + const schema = await chartSchema({ + ...CHART, + series: [{ name: 'forecast_amount', color: '#f00' }], + }); + expect(schema.series).toHaveLength(1); + expect(schema.series[0].dataKey).toBe('amount'); + expect(schema.series[0].color).toBeUndefined(); + }); + + it('the FIRST entry naming the measure wins over a later duplicate', async () => { + const schema = await chartSchema({ + ...CHART, + series: [ + { name: 'amount', color: '#f00' }, + { name: 'amount', color: '#0f0' }, + ], + }); + expect(schema.series[0].color).toBe('#f00'); + }); + + it('an out-of-family `type` does not become a per-series mark', async () => { + // `pie` composes with nothing on a cartesian plot; taken literally it would + // count as a family disagreement, flip the chart to a combo, and then draw + // the series as a bar anyway. + const schema = await chartSchema({ ...CHART, series: [{ name: 'amount', type: 'pie' }] }); + expect(schema.series[0].chartType).toBeUndefined(); + }); + + it('presentation does not displace the #4020 display name', async () => { + // The two cards meet on this line: the merge carries `color`/`stack`, and + // the LABEL still comes from #4020's three-level resolution. + const schema = await chartSchema({ + ...CHART, + series: [{ name: 'amount', color: '#f00', label: '金额' }], + }); + expect(schema.series[0]).toMatchObject({ color: '#f00', label: '金额' }); + }); + + it('a chart authoring no series keeps the derived binding untouched', async () => { + const schema = await chartSchema({ ...CHART }); + expect(schema.series).toEqual([{ dataKey: 'amount', label: 'Amount' }]); + }); +}); diff --git a/packages/plugin-report/src/__tests__/DatasetReportRenderer.chartNullCategory.test.tsx b/packages/plugin-report/src/__tests__/DatasetReportRenderer.chartNullCategory.test.tsx new file mode 100644 index 0000000000..289e51f4c7 --- /dev/null +++ b/packages/plugin-report/src/__tests__/DatasetReportRenderer.chartNullCategory.test.tsx @@ -0,0 +1,269 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#4878 — a report's embedded chart never bucketed a NULL category. + * + * `DatasetReportChart` built its rows as `relabelDimensions(state.rows, …)` and + * handed them to the registered chart component verbatim. Nothing on that path + * bucketed a null dimension value, so the renderer received a null category — + * the exact input objectui#4466 measured as drawing **no mark at all**. The + * dashboard and chart-view surfaces never had the defect because they route + * through `buildChartSeries` (`@object-ui/core`), whose `bucketNullCategories` + * limb is where #4466 / #4497 / #4673 / #4500 / #4508 were all fixed. + * + * The fix ROUTES rather than re-derives, so this file pins the family as + * INHERITED properties, not as a second implementation of them: + * + * ① the bucket exists and keeps its measure (#4466); + * ② its label comes from the locale bundle at this call site, because + * `@object-ui/core` is React-free (#4500); + * ③ a stored value that literally spells the bucket label stays a DIFFERENT + * bucket (#4508); + * ④ #4020's three-level measure display name still outranks the label the + * derivation itself assigns — the load-bearing constraint of the switch. + * + * DIRECTIONS, written before the reverse verification was run. The mutation is + * "put the deleted limb back": restore `data: relabelDimensions(state.rows, …)` + * and `series: [{ dataKey: yAxis, label: measureLabel }]`, i.e. the pre-change + * lines. + * + * Measured: 5 red / 5 green, matching the prediction case for case. + * + * - ①/②/③ go RED: with the rows passed through, the category stays `null` and + * every assertion about a bucket string fails on `null`. This is the + * ordinary direction and the one the card describes. + * - The `en` BOUNDARY case goes red too, and that is worth stating precisely + * rather than filing under "boundary cases stay green". It is green on both + * sides of the NARROWER mutation it was written for — dropping the + * `nullCategoryLabel` option while keeping the routing, i.e. objectui#4500's + * own limb, where core's English floor and the `en` pack produce the same + * bytes through different channels. It cannot be green across THIS mutation, + * which removes the bucket itself: there is no label to read in either + * language when nothing is bucketed. + * - The no-null-group BOUNDARY stays GREEN on both sides, and that is its job: + * this card buys a bucketed null category and must not move a byte of an + * ordinary report's chart. + * - ④ stays GREEN on both sides too, and is NOT a weak case: it is the guard + * that the new derivation's own `fields[].label` did not silently take over + * the label, which is a failure only visible AFTER the change. Reverting + * cannot show it — the pre-change code had no derivation to be outranked by. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, cleanup, waitFor } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ComponentRegistry, NULL_CATEGORY_LABEL, chartRowBucketId } from '@object-ui/core'; +import { I18nProvider } from '@object-ui/i18n'; +import { DatasetReportRenderer } from '../DatasetReportRenderer'; + +/** Props the registered chart component was handed, or `null` if never called. */ +let captured: { schema: Record } | null = null; + +beforeEach(() => { + captured = null; + ComponentRegistry.register('chart', (props: any) => { + captured = props; + return null; + }); +}); + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +/** + * The card's own measured shape: a dominant NULL group beside a named one. + * `owner` is a plain text dimension on purpose — this card is about the null + * BUCKET, so the fixture carries no option list that could relabel a category + * through `relabelDimensions` and blur which channel produced the string. + */ +const NULL_GROUP = { + rows: [ + { owner: null, n: 51 }, + { owner: 'Dev Admin', n: 2 }, + ], + fields: [ + { name: 'owner', type: 'text', label: 'Owner' }, + { name: 'n', type: 'number', label: 'Events' }, + ], +}; + +/** The same report over data with NO null group — the untouched-surface case. */ +const FULLY_KEYED = { + rows: [ + { owner: 'Ada Lovelace', n: 51 }, + { owner: 'Dev Admin', n: 2 }, + ], + fields: NULL_GROUP.fields, +}; + +/** + * objectui#4508's collision, reachable here for the first time: a stored value + * that literally spells the bucket label, beside the real null group. + */ +const SPELLS_THE_LABEL = { + rows: [ + { owner: null, n: 51 }, + { owner: NULL_CATEGORY_LABEL, n: 7 }, + ], + fields: NULL_GROUP.fields, +}; + +const sourceOf = (result: unknown) => ({ queryDataset: vi.fn(async () => result) }); + +const BASE = { + name: 'events_by_owner', + type: 'tabular', + dataset: 'event_metrics', + rows: ['owner'], + values: ['n'], + chart: { type: 'bar', xAxis: 'owner', yAxis: 'n' }, +}; + +/** Render the report, optionally inside a locale, and wait for the chart props. */ +async function chartSchema( + result: unknown, + opts: { language?: string; report?: Record } = {}, +) { + const element = ( + + ); + render( + opts.language ? ( + + {element} + + ) : ( + element + ), + ); + await waitFor(() => expect(captured?.schema?.data?.length).toBe(2)); + return captured!.schema; +} + +/** The categories the report actually handed the chart component. */ +const categoriesOf = (schema: Record) => schema.data.map((r: any) => r.owner); + +describe('report chart — the null category is bucketed (objectui#4878 / #4466)', () => { + it('① replaces the raw null with the bucket label instead of forwarding it', async () => { + const schema = await chartSchema(NULL_GROUP); + // The card's measurement: `data[0].owner` was `null` and drew no mark. + expect(categoriesOf(schema)).toContain(NULL_CATEGORY_LABEL); + expect(categoriesOf(schema)).not.toContain(null); + }); + + it('① keeps the measure attached to the bucket — the mark has a height', async () => { + // #4466's harm was not an empty chart but a quietly WRONG one: the dominant + // group vanished while the y-axis still accommodated it. The bucket must + // therefore carry its own 51, not merely exist. + const schema = await chartSchema(NULL_GROUP); + const byCategory = Object.fromEntries(schema.data.map((r: any) => [r.owner, r.n])); + expect(byCategory).toMatchObject({ [NULL_CATEGORY_LABEL]: 51, 'Dev Admin': 2 }); + }); + + it('① the series still binds the plotted measure, so the bucket draws', async () => { + // A bucket with no series bound to it is objectui#4673's defect one axis + // over: the number is in the row and no mark reads it. + const schema = await chartSchema(NULL_GROUP); + expect(schema.series.map((s: any) => s.dataKey)).toEqual(['n']); + expect(schema.xAxisKey).toBe('owner'); + }); +}); + +describe('report chart — the bucket label comes from the locale bundle (objectui#4500)', () => { + it('② reads `(未指定)` under `zh`, not the English floor', async () => { + // `@object-ui/core` is React-free and cannot read the bundle, so a call site + // that passes no `nullCategoryLabel` gets the English constant — a zh + // console would draw the bar and label it `(None)`. + const schema = await chartSchema(NULL_GROUP, { language: 'zh' }); + expect(categoriesOf(schema)).toContain('(未指定)'); + expect(categoriesOf(schema)).not.toContain(NULL_CATEGORY_LABEL); + }); + + it('BOUNDARY — under `en` the bucket reads `(None)`, through the same channel', async () => { + // The boundary this guards is objectui#4500's CHANNEL, not #4878's routing: + // core's hardcoded floor produces this string without the option and the + // `en` pack's `chart.nullCategory` produces it with, so an `en` console + // reads exactly what it read before the label became localizable. Measured + // red under the routing mutation, as it must be — with no bucket at all + // there is no label to read in either language. + const schema = await chartSchema(NULL_GROUP, { language: 'en' }); + expect(categoriesOf(schema)).toContain(NULL_CATEGORY_LABEL); + expect(categoriesOf(schema)).not.toContain('(未指定)'); + }); +}); + +describe('report chart — bucket IDENTITY is not the bucket label (objectui#4508)', () => { + it('③ a stored value spelling `(None)` stays a different bucket from null', async () => { + const schema = await chartSchema(SPELLS_THE_LABEL); + // Two rows in, two rows out: the null group and the record whose stored + // value happens to spell its label were never the same group. + expect(schema.data).toHaveLength(2); + const ids = schema.data.map((r: any) => chartRowBucketId(r)); + expect(ids.filter(Boolean)).toHaveLength(2); + expect(new Set(ids).size).toBe(2); + // Both paint the same axis text, which is exactly why they need identities. + expect(categoriesOf(schema)).toEqual([NULL_CATEGORY_LABEL, NULL_CATEGORY_LABEL]); + }); +}); + +describe('report chart — the untouched surface (objectui#4878 boundary)', () => { + it('BOUNDARY — a report with no null group is byte-identical', async () => { + const schema = await chartSchema(FULLY_KEYED); + expect(schema.data).toEqual([ + { owner: 'Ada Lovelace', n: 51 }, + { owner: 'Dev Admin', n: 2 }, + ]); + // No bucket identity is written when the display string names the bucket on + // its own — the rows a chart is drawn from are an authoring-visible surface. + expect(schema.data.map((r: any) => chartRowBucketId(r))).toEqual([undefined, undefined]); + }); +}); + +describe('report chart — #4020 still outranks the derivation (the load-bearing constraint)', () => { + it('④ an authored chart.series[].label beats the derived `fields[].label`', async () => { + // `buildChartSeries` assigns `label: fields.find(…)?.label ?? name`, i.e. + // `Events` here. objectui#4020's resolution is HIGHER: the authored + // per-chart override first, then the dataset label through the i18n + // field-label convention. Adopting the derivation must not demote it. + const schema = await chartSchema(NULL_GROUP, { + report: { + chart: { + type: 'bar', + xAxis: 'owner', + yAxis: 'n', + series: [{ name: 'n', label: '事件数' }], + }, + }, + }); + expect(schema.series[0].label).toBe('事件数'); + }); + + it('④ an i18n label record still picks the console language, not the first limb', async () => { + // The derivation cannot do this: core holds no provider, so its own label + // pick is first-string-wins and would paint `Events (en)` on a zh console. + const schema = await chartSchema(NULL_GROUP, { + language: 'zh', + report: { + chart: { + type: 'bar', + xAxis: 'owner', + yAxis: 'n', + series: [{ name: 'n', label: { en: 'Events (en)', 'zh-CN': '事件数' } }], + }, + }, + }); + expect(schema.series[0].label).toBe('事件数'); + }); + + it('④ with nothing authored, the dataset measure label still wins', async () => { + const schema = await chartSchema(NULL_GROUP); + expect(schema.series[0].label).toBe('Events'); + }); +});