diff --git a/.changeset/objectchart-honors-chartconfig.md b/.changeset/objectchart-honors-chartconfig.md new file mode 100644 index 0000000000..1132b19c98 --- /dev/null +++ b/.changeset/objectchart-honors-chartconfig.md @@ -0,0 +1,50 @@ +--- +"@object-ui/plugin-charts": minor +"@object-ui/components": patch +--- + +feat(charts): ObjectChart honors the spec `ChartConfig` author shape (objectui#2880 / framework#3729) + +`ChartConfigSchema` is the chart protocol, but the renderer only ever read a +Recharts-flavoured internal shape — `chartType`, `xAxisKey`, `series[].dataKey`. +Everything an author wrote in the SPEC shape reached the renderer and was +silently dropped, which is exactly what ADR-0078 forbids. framework#3725 +documented the gap by trimming the published contract down to the props that +actually worked; this closes it the other way round. + +**S1 — one normalization boundary.** `normalizeChartSchema` translates the +author shape into the internal pipeline contract in a single place, rather than +scattering `??` fallbacks through the render tree (framework PD #12: one +translation is a contract mapping, N fallbacks are a second dialect): + +- `type` → `chartType`, `xAxis: { field }` → `xAxisKey`, `series: [{ name }]` → + `series: [{ dataKey }]` +- the report surface's bare-string `xAxis`/`yAxis` resolve too +- `yAxis: [{ field }]` alone plots, with no `series` declared +- **internal props win**, so `DashboardRenderer`, `ObjectView` and the dataset + path are byte-for-byte unaffected — there is no migration + +**The `type` collision.** `ChartConfig.type` is the chart family, but on any +surface that flattens chart config into a props bag `type` is already the SDUI +envelope's component discriminator. Spreading props last let an author's +`type="bar"` replace `object-chart` so the block stopped resolving; stamping the +discriminator last ate the author's value instead. The react-page wrapper now +keeps both: the discriminator wins the `type` slot and the author's value is +preserved beside it as `specType`, which the normalizer reads back. + +**S2 — axis presentation.** `ChartAxis.format` drives the tick formatter (via +`Intl.NumberFormat`, no new dependency), `min`/`max` pin the domain, +`logarithmic` swaps the scale, `title` labels the axis, and `showGridLines` is +honored. A second `yAxis` entry (or `position: 'right'`) turns on the secondary +axis that `series[].yAxis` binds to — in combo charts an explicit binding now +beats the family-derived bar→left/line→right guess. `showLegend` is honored, +and `title`/`subtitle` render above the plot instead of only titling the +drill-down drawer. + +**S3 — `series[].stack`, `annotations`, `interaction`.** Stacking passes the +author's group name through as Recharts' `stackId`. Annotations render as +`ReferenceLine` (`type: 'line'`) / `ReferenceArea` (`type: 'region'`) with the +declared axis, colour, style and label. `interaction.tooltips: false` suppresses +the hover card and `interaction.brush: true` adds the range selector; +`showDataLabels` prints values on the marks. `interaction.zoom` has no Recharts +primitive behind it and is deliberately still unimplemented rather than faked. diff --git a/packages/components/src/renderers/layout/react-page.tsx b/packages/components/src/renderers/layout/react-page.tsx index 290657cb32..c31f588038 100644 --- a/packages/components/src/renderers/layout/react-page.tsx +++ b/packages/components/src/renderers/layout/react-page.tsx @@ -59,8 +59,21 @@ function buildComponentScope(dataSource: unknown): Record = ({ children: _children, ...props }) => - React.createElement(SchemaRenderer as any, { schema: { type: tag, dataSource, ...props } }); + // `type` is the SDUI envelope's component discriminator, but it is ALSO a + // legitimate prop name in a block's own spec schema — `ChartConfig.type` is + // the chart family. Flattening props into the schema bag collides the two: + // spreading last let an author's `type="bar"` replace `object-chart` and + // the block stopped resolving; stamping the discriminator last silently ate + // the author's value instead. Neither is acceptable (ADR-0078), so the + // discriminator wins the `type` slot and the author's value is preserved + // beside it under `specType` for the block to read + // (objectui#2880 / framework#3729). + const Wrapper: React.FC = ({ children: _children, ...props }) => { + const specType = typeof props.type === 'string' && props.type !== tag ? props.type : undefined; + return React.createElement(SchemaRenderer as any, { + schema: { dataSource, ...props, ...(specType ? { specType } : {}), type: tag }, + }); + }; Wrapper.displayName = name; scope[name] = Wrapper; } diff --git a/packages/plugin-charts/src/AdvancedChartImpl.specConfig.test.tsx b/packages/plugin-charts/src/AdvancedChartImpl.specConfig.test.tsx new file mode 100644 index 0000000000..0867fdb106 --- /dev/null +++ b/packages/plugin-charts/src/AdvancedChartImpl.specConfig.test.tsx @@ -0,0 +1,164 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * framework#3729 / objectui#2880 — the spec's `ChartConfig` presentation props + * are honored, not silently dropped. + * + * `showLegend`, `title`/`subtitle`, `yAxis[].min/max/format/logarithmic`, + * `series[].stack`, `series[].yAxis`, `annotations` and `interaction` were all + * declared by `ChartConfigSchema`, reached this renderer, and did nothing — + * exactly the silently-inert metadata ADR-0078 forbids. These pin the wiring + * at the DOM/prop level so a future refactor cannot quietly drop them again. + */ + +import React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, cleanup, screen } from '@testing-library/react'; + +// Recharts' ResponsiveContainer measures via ResizeObserver, which reports 0×0 +// under the headless DOM, so nothing paints. Fix its size. +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: any) => + React.cloneElement(children, { width: 480, height: 320 }), + }; +}); + +import AdvancedChartImpl from './AdvancedChartImpl'; + +afterEach(cleanup); + +const DATA = [ + { status: 'open', total: 120, rate: 0.4 }, + { status: 'paid', total: 80, rate: 0.9 }, +]; + +const base = { + chartType: 'bar' as const, + data: DATA, + xAxisKey: 'status', + series: [{ dataKey: 'total' }], + isAnimationActive: false, +}; + +describe('AdvancedChartImpl — spec ChartConfig chrome', () => { + it('renders title and subtitle above the plot', () => { + render(); + expect(screen.getByText('Invoice value')).toBeTruthy(); + expect(screen.getByText('by status')).toBeTruthy(); + }); + + it('adds no wrapper chrome when neither is set', () => { + const { container } = render(); + // ChartFrame is a passthrough with no title/subtitle, so the container's + // first child is still the chart itself. + expect(container.querySelector('.text-muted-foreground')).toBeNull(); + }); + + it('honors showLegend: false', () => { + const { container: withLegend } = render(); + const legendCount = withLegend.querySelectorAll('.recharts-legend-wrapper').length; + cleanup(); + const { container: without } = render(); + expect(without.querySelectorAll('.recharts-legend-wrapper').length).toBeLessThan(legendCount); + }); +}); + +describe('AdvancedChartImpl — spec ChartAxis', () => { + it('formats y ticks with the declared format instead of the compact default', () => { + const { container } = render( + , + ); + // Recharts 3 draws tick labels in their own z-index layer rather than + // inside the axis group, so read every and look for the currency + // symbol — the compact default would render a bare "120". + const texts = Array.from(container.querySelectorAll('text')).map((t) => t.textContent ?? ''); + expect(texts.some((t) => t.includes('$'))).toBe(true); + }); + + it('renders an axis title', () => { + render(); + expect(screen.getByText('Revenue')).toBeTruthy(); + }); + + it('draws a second y-axis when two are declared', () => { + const { container: single } = render(); + const singleCount = single.querySelectorAll('.recharts-yAxis').length; + cleanup(); + const { container: dual } = render( + , + ); + expect(dual.querySelectorAll('.recharts-yAxis').length).toBeGreaterThan(singleCount); + }); +}); + +describe('AdvancedChartImpl — spec series.stack', () => { + /** X offsets of every drawn bar (Recharts 3 draws bars as ). */ + const barXs = (container: HTMLElement) => + Array.from(container.querySelectorAll('.recharts-bar-rectangle path')).map((p) => p.getAttribute('x')); + + it('stacks series sharing a stack group', () => { + // Recharts stacks by `stackId`. Stacked: both series of a category share + // one column band → 2 distinct x for 2 categories × 2 series. Grouped: + // the bars sit side by side → 4 distinct x. + const { container: stacked } = render( + , + ); + const stackedXs = barXs(stacked); + expect(stackedXs.length).toBe(4); + expect(new Set(stackedXs).size).toBe(2); + cleanup(); + + const { container: grouped } = render( + , + ); + expect(new Set(barXs(grouped)).size).toBe(4); + }); +}); + +describe('AdvancedChartImpl — spec annotations', () => { + it('draws a reference line for a line annotation', () => { + const { container } = render( + , + ); + expect(container.querySelectorAll('.recharts-reference-line').length).toBeGreaterThan(0); + expect(screen.getByText('Target')).toBeTruthy(); + }); + + it('draws a reference area for a region annotation', () => { + const { container } = render( + , + ); + expect(container.querySelectorAll('.recharts-reference-area').length).toBeGreaterThan(0); + }); + + it('draws nothing extra when no annotation is declared', () => { + const { container } = render(); + expect(container.querySelectorAll('.recharts-reference-line').length).toBe(0); + }); +}); + +describe('AdvancedChartImpl — spec interaction', () => { + it('adds the brush when interaction.brush is on', () => { + const { container } = render(); + expect(container.querySelectorAll('.recharts-brush').length).toBeGreaterThan(0); + }); + + it('omits the brush by default', () => { + const { container } = render(); + expect(container.querySelectorAll('.recharts-brush').length).toBe(0); + }); +}); diff --git a/packages/plugin-charts/src/AdvancedChartImpl.tsx b/packages/plugin-charts/src/AdvancedChartImpl.tsx index 0e2feddfc4..e8b1f9d491 100644 --- a/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -34,6 +34,9 @@ import { Treemap, Sankey, Tooltip, + ReferenceLine, + ReferenceArea, + Brush, } from 'recharts'; import { ChartContainer, @@ -44,6 +47,7 @@ import { ChartConfig } from './ChartContainerImpl'; import { mapScatterClick, mapTreemapClick, mapSankeyClick } from './chartDrillEvents'; +import { formatterFor, domainFor, type NormalizedAxis, type NormalizedSeries } from './normalizeChartSchema'; import { buildCategoryRank } from '@object-ui/core'; // Default color fallback for chart series @@ -96,7 +100,11 @@ export interface AdvancedChartImplProps { data?: Array>; config?: ChartConfig; xAxisKey?: string; - series?: Array<{ dataKey: string; chartType?: 'bar' | 'line' | 'area'; variant?: 'current' | 'comparison'; opacity?: number; dashArray?: string; label?: string }>; + /** + * Plotted series, in the renderer's internal shape. Authors write the spec + * `series: [{ name, stack, yAxis, … }]`; `normalizeChartSchema` translates. + */ + series?: NormalizedSeries[]; className?: string; /** Categorical/series colour palette. Overrides the theme's `--chart-1..n` * defaults so a page/dashboard can brand its charts (data-screens). */ @@ -130,6 +138,32 @@ export interface AdvancedChartImplProps { * area/pie/donut. Other chart types are no-ops in L1. */ onChartClick?: (event: { category?: string; series?: string; value?: number }) => void; + /** + * Spec `ChartAxis` presentation for the category axis — `format` (tick + * formatter), `title`, `showGridLines`. Its `field` already arrived as + * {@link AdvancedChartImplProps.xAxisKey}. Resolved by + * `normalizeChartSchema`, so the renderer never parses the author shape. + */ + xAxis?: NormalizedAxis; + /** + * Spec `ChartConfig.yAxis` — one entry per value axis, in declaration order. + * Index 0 is the primary axis; a second entry (or one with + * `position: 'right'`) turns on the secondary axis that `series[].yAxis` + * binds to. Carries `min`/`max` (domain), `format` (ticks), `logarithmic` + * (scale) and `title`. + */ + yAxes?: NormalizedAxis[]; + /** Spec `ChartConfig.showLegend`. Omitted → shown (the schema default). */ + showLegend?: boolean; + /** Spec `ChartConfig.showDataLabels` — print each point's value on the mark. */ + showDataLabels?: boolean; + /** Spec `ChartConfig.title` / `.subtitle`, rendered above the plot. */ + title?: string; + subtitle?: string; + /** Spec `ChartConfig.annotations` — reference lines / bands. */ + annotations?: Array>; + /** Spec `ChartConfig.interaction` — `tooltips`, `brush`. */ + interaction?: Record; /** * Disable Recharts' entrance animation when `false`. Animations drive the * reveal via `requestAnimationFrame`, which browsers throttle/pause in @@ -162,6 +196,26 @@ function TreemapCell(props: any) { ); } +/** + * Renders the spec's `ChartConfig.title` / `.subtitle` above the plot. + * + * Both were previously accepted by the contract and drawn by nothing — `title` + * only ever reached the drill-down drawer's heading. With neither set this + * returns the chart untouched, so no existing caller gains a wrapper element. + */ +function ChartFrame({ title, subtitle, children }: { title?: string; subtitle?: string; children: React.ReactNode }) { + if (!title && !subtitle) return <>{children}; + return ( +
+
+ {title ?
{title}
: null} + {subtitle ?
{subtitle}
: null} +
+
{children}
+
+ ); +} + /** * AdvancedChartImpl - The heavy implementation that imports Recharts with full features * This component is lazy-loaded to avoid including Recharts in the initial bundle @@ -178,6 +232,14 @@ export default function AdvancedChartImpl({ categoryOrder, onChartClick, isAnimationActive, + xAxis: xAxisSpec, + yAxes, + showLegend, + showDataLabels, + title, + subtitle, + annotations, + interaction, }: AdvancedChartImplProps) { // Normalize 'column' → 'bar' (Recharts BarChart is already vertical). // 'column' is the spec-level alias for vertical bars; 'horizontal-bar' stays as-is. @@ -343,6 +405,118 @@ export default function AdvancedChartImpl({ } }, []); + // ── Spec ChartAxis presentation (objectui#2880 S2) ────────────────────── + // The author's `format` wins over the compact default; `min`/`max` pin the + // domain; `logarithmic` swaps the scale; `title` labels the axis. All three + // arrive already parsed from `normalizeChartSchema`, so nothing here has to + // know the author-facing shape. + const primaryY: NormalizedAxis | undefined = yAxes?.[0]; + // A secondary axis exists when a second entry is declared, or the only entry + // asks to sit on the right. + const secondaryY: NormalizedAxis | undefined = + yAxes && yAxes.length > 1 + ? yAxes[1] + : primaryY?.position === 'right' + ? primaryY + : undefined; + const hasDualAxis = !!yAxes && (yAxes.length > 1); + + const xTickFormatter = React.useMemo( + () => formatterFor(xAxisSpec?.format) ?? formatTick, + [xAxisSpec?.format, formatTick], + ); + const yTickFormatter = React.useMemo( + () => formatterFor(primaryY?.format) ?? formatYTick, + [primaryY?.format, formatYTick], + ); + const y2TickFormatter = React.useMemo( + () => formatterFor(secondaryY?.format) ?? formatYTick, + [secondaryY?.format, formatYTick], + ); + + /** Recharts props derived from one spec y-axis (domain / scale / label). */ + const yAxisSpecProps = React.useCallback((axis: NormalizedAxis | undefined) => { + if (!axis) return {}; + const domain = domainFor(axis); + return { + ...(domain ? { domain } : {}), + // `allowDataOverflow` is what makes an explicit domain actually clip + // rather than being silently widened to fit the data. + ...(domain ? { allowDataOverflow: true } : {}), + ...(axis.logarithmic ? { scale: 'log' as const, domain: domain ?? ([1, 'auto'] as any) } : {}), + ...(axis.title ? { label: { value: axis.title, angle: -90, position: 'insideLeft' as const } } : {}), + }; + }, []); + + // `showGridLines` is per-axis in the spec; the renderer draws one grid, so + // an explicit `false` on EITHER axis turns off that axis's lines. + const showYGrid = primaryY?.showGridLines !== false; + // The default grid draws horizontal lines only (`vertical={false}`), which is + // the right default for a categorical x-axis; honour an explicit opt-in. + const gridProps = { + vertical: xAxisSpec?.showGridLines === true, + horizontal: showYGrid, + }; + + // Legend is on unless the author turned it off (spec default `true`). + const legendVisible = showLegend !== false; + + // ── Spec ChartConfig.annotations (objectui#2880 S3) ───────────────────── + // `type: 'line'` → ReferenceLine at `value`; `type: 'region'` → ReferenceArea + // spanning `value`..`endValue`. `axis` picks which axis the value belongs to. + const annotationEls = React.useMemo(() => { + if (!Array.isArray(annotations) || annotations.length === 0) return null; + const DASH: Record = { solid: undefined, dashed: '4 4', dotted: '1 4' }; + return annotations.map((a, i) => { + const onX = a?.axis === 'x'; + const stroke = resolveColor(String(a?.color || 'hsl(var(--muted-foreground))')); + const strokeDasharray = DASH[String(a?.style ?? 'dashed')]; + const text = typeof a?.label === 'string' ? a.label : undefined; + const labelProp = text ? { label: { value: text, position: 'insideTopRight' as const, fill: stroke, fontSize: 11 } } : {}; + if (a?.type === 'region') { + const range = onX ? { x1: a.value, x2: a.endValue } : { y1: a.value, y2: a.endValue }; + return ( + + ); + } + return ( + + ); + }); + }, [annotations, hasDualAxis]); + + // ── Spec ChartConfig.interaction (objectui#2880 S3) ───────────────────── + // `tooltips: false` suppresses the hover card; `brush: true` adds the range + // selector under the plot. `zoom` has no Recharts primitive behind it and is + // deliberately not faked — see the note in ChartConfigSchema. + const tooltipsEnabled = interaction?.tooltips !== false; + const brushEnabled = interaction?.brush === true; + const brushEl = brushEnabled + ? + : null; + + /** `showDataLabels` prints each point's value on the mark itself. */ + const dataLabel = (formatter?: (v: any) => string) => + showDataLabels + ? + : null; + // Shared X-axis props for time/categorical axes. Recharts' `minTickGap` // automatically thins ticks that would otherwise overlap, so we no // longer hard-code `interval={0}` (which forced every label and @@ -353,9 +527,10 @@ export default function AdvancedChartImpl({ axisLine: false as const, interval: 'preserveStartEnd' as const, minTickGap: isMobile ? 32 : 48, - tickFormatter: formatTick, + tickFormatter: xTickFormatter, + ...(xAxisSpec?.title ? { label: { value: xAxisSpec.title, position: 'insideBottom' as const, offset: -4 } } : {}), ...(!isMobile && hasLongLabels && { angle: -35, textAnchor: 'end' as const, height: 60 }), - }), [isMobile, hasLongLabels, formatTick]); + }), [isMobile, hasLongLabels, xTickFormatter, xAxisSpec?.title]); // Pie and Donut charts if (chartType === 'pie' || chartType === 'donut') { @@ -604,33 +779,57 @@ export default function AdvancedChartImpl({ // Combo chart (mixed bar + line on same chart) if (chartType === 'combo') { return ( + - + - - - } /> - } - {...(isMobile && { verticalAlign: "bottom", wrapperStyle: { fontSize: '11px', paddingTop: '8px' } })} - /> + + + {tooltipsEnabled ? } /> : null} + {legendVisible ? ( + } + {...(isMobile && { verticalAlign: "bottom", wrapperStyle: { fontSize: '11px', paddingTop: '8px' } })} + /> + ) : null} + {annotationEls} + {brushEl} {series.map((s: any, index: number) => { const color = resolveColor(config[s.dataKey]?.color || DEFAULT_CHART_COLOR); const seriesType = s.chartType || (index === 0 ? 'bar' : 'line'); - const yAxisId = seriesType === 'bar' ? 'left' : 'right'; + // An explicit spec `series[].yAxis` wins over the family-derived + // default (bar→left / line→right), which is only a guess. + const yAxisId = s.yAxis === 'right' || s.yAxis === 'left' + ? s.yAxis + : (seriesType === 'bar' ? 'left' : 'right'); const cmp = comparisonStyle(s, seriesType as any); + const stackProps = s.stack ? { stackId: String(s.stack) } : {}; + const valueFormatter = formatterFor((yAxisId === 'right' ? secondaryY : primaryY)?.format); if (seriesType === 'line') { - return ; + return ( + + {dataLabel(valueFormatter)} + + ); } if (seriesType === 'area') { - return ; + return ( + + {dataLabel(valueFormatter)} + + ); } - return ; + return ( + + {dataLabel(valueFormatter)} + + ); })} + ); } @@ -652,6 +851,7 @@ export default function AdvancedChartImpl({ const gslug = (c: string) => 'g' + c.replace(/[^a-zA-Z0-9]/g, ''); return ( + @@ -668,30 +868,54 @@ export default function AdvancedChartImpl({ ))} - + {isHorizontal ? ( <> - + {/* Horizontal bars swap the axis roles: the VALUE axis is x, so the + spec y-axis config (domain/format/scale) applies to it. */} + String(d[xAxisKey] ?? '').length)) * 7))} - tickFormatter={formatTick} + tickFormatter={xTickFormatter} /> ) : ( <> - + + {hasDualAxis ? ( + + ) : null} )} - } /> - } - {...(isMobile && { verticalAlign: "bottom", wrapperStyle: { fontSize: '11px', paddingTop: '8px' } })} - /> + {tooltipsEnabled ? } /> : null} + {legendVisible ? ( + } + {...(isMobile && { verticalAlign: "bottom", wrapperStyle: { fontSize: '11px', paddingTop: '8px' } })} + /> + ) : null} + {annotationEls} + {brushEl} {series.map((s: any, sIdx: number) => { const palette = getPalette(); // Comparison series should mirror the color of the primary series @@ -704,6 +928,17 @@ export default function AdvancedChartImpl({ const baseIdx = isComparison ? series.indexOf(baseSeries) : sIdx; const seriesColor = resolveColor(config[baseSeries.dataKey]?.color || palette[baseIdx % palette.length] || DEFAULT_CHART_COLOR); + // Spec `series[].yAxis` binds this series to the secondary axis. + // Only meaningful once a second axis is declared. + const axisProps = hasDualAxis ? { yAxisId: s.yAxis === 'right' ? 'right' : 'left' } : {}; + // Spec `series[].stack` — series sharing a group id stack together. + // Recharts keys stacking off `stackId`, so the author's group name + // passes through unchanged. + const stackProps = s.stack ? { stackId: String(s.stack) } : {}; + const valueFormatter = formatterFor( + (s.yAxis === 'right' ? secondaryY : primaryY)?.format, + ); + if (chartType === 'bar' || chartType === 'horizontal-bar') { // For categorical bar charts with a single primary series, // color each bar distinctly. With a comparison overlay the @@ -713,24 +948,34 @@ export default function AdvancedChartImpl({ const colorPerCategory = primaryCount === 1 && !isComparison && series.length === 1 && data.length > 1; const cmp = comparisonStyle(s, 'bar'); return ( - + {colorPerCategory && data.map((entry, idx) => ( ))} + {dataLabel(valueFormatter)} ); } if (chartType === 'line') { const cmp = comparisonStyle(s, 'line'); - return ; + return ( + + {dataLabel(valueFormatter)} + + ); } if (chartType === 'area') { const cmp = comparisonStyle(s, 'area'); - return ; + return ( + + {dataLabel(valueFormatter)} + + ); } return null; })} + ); } diff --git a/packages/plugin-charts/src/ChartRenderer.tsx b/packages/plugin-charts/src/ChartRenderer.tsx index 88b546bb58..e07ad45340 100644 --- a/packages/plugin-charts/src/ChartRenderer.tsx +++ b/packages/plugin-charts/src/ChartRenderer.tsx @@ -2,6 +2,7 @@ import React, { Suspense } from 'react'; import { Skeleton } from '@object-ui/components'; import type { ChartConfig } from './ChartContainerImpl'; +import { normalizeChartSchema } from './normalizeChartSchema'; // 🚀 Lazy load the implementation files const LazyChart = React.lazy(() => import('./ChartImpl')); @@ -46,8 +47,25 @@ export interface ChartRendererProps { chartType?: 'bar' | 'column' | 'horizontal-bar' | 'line' | 'area' | 'pie' | 'donut' | 'radar' | 'scatter' | 'funnel' | 'combo' | 'treemap' | 'sankey'; data?: Array>; config?: Record; + /** Internal binding. Authors write the spec `xAxis: { field }` (or the + * report surface's bare string); `normalizeChartSchema` resolves both. */ xAxisKey?: string; - series?: Array<{ dataKey: string; label?: string; variant?: 'current' | 'comparison'; opacity?: number; dashArray?: string; chartType?: 'bar' | 'line' | 'area' }>; + /** Internal binding. Authors write the spec `series: [{ name, … }]`. */ + series?: Array<{ dataKey: string; label?: string; variant?: 'current' | 'comparison'; opacity?: number; dashArray?: string; chartType?: 'bar' | 'line' | 'area'; stack?: string; yAxis?: 'left' | 'right'; color?: string }>; + /** Spec `ChartConfig` shape — honored via `normalizeChartSchema` + * (objectui#2880). Listed here so the author-facing contract type-checks; + * the internal props above win when both are present. */ + xAxis?: unknown; + yAxis?: unknown; + showLegend?: boolean; + showDataLabels?: boolean; + title?: unknown; + subtitle?: unknown; + annotations?: Array>; + interaction?: Record; + /** An author `type` rescued from the SDUI envelope's discriminator + * collision by the react-page wrapper — see `normalizeChartSchema`. */ + specType?: string; colors?: string[]; /** Per-category colour map (value/label → colour). Wins over `colors` per * category; see AdvancedChartImpl. Set by ObjectChart from select/lookup @@ -70,14 +88,22 @@ export interface ChartRendererProps { * ChartRenderer - The public API for the advanced chart component */ export const ChartRenderer: React.FC = ({ schema, onChartClick }) => { - // ⚡️ Adapter: Normalize JSON schema to Recharts Props + // ⚡️ Adapter: Normalize the AUTHOR-facing chart schema to Recharts props. + // + // The spec shape (`type` / `xAxis: {field}` / `yAxis: [{field, format, …}]` / + // `series: [{name, stack, yAxis}]`) and the renderer's internal shape + // (`chartType` / `xAxisKey` / `series: [{dataKey}]`) both land here; + // `normalizeChartSchema` is the single translation point, with internal props + // winning so DashboardRenderer/ObjectView/the dataset path are untouched + // (objectui#2880 S1). const props = React.useMemo(() => { - // 1. Defaults - let series = schema.series; - let xAxisKey = schema.xAxisKey; + const spec = normalizeChartSchema(schema); + + let series: any[] | undefined = schema.series ?? spec.series; + let xAxisKey = schema.xAxisKey ?? spec.xAxisKey; let config = schema.config; - // 2. Adapt Tremor/Simple format (categories -> series, index -> xAxisKey) + // Adapt the Tremor/simple format (categories -> series, index -> xAxisKey) if (!xAxisKey) { if ((schema as any).index) xAxisKey = (schema as any).index; else if ((schema as any).category) xAxisKey = (schema as any).category; // Support Pie/Donut category @@ -91,24 +117,26 @@ export const ChartRenderer: React.FC = ({ schema, onChartCli series = [{ dataKey: (schema as any).value }]; } } - - // 3. Auto-generate config/colors if missing + + // Auto-generate config/colors if missing. A spec `series[].color` is an + // explicit author choice, so it wins over the positional palette. if (!config && series) { - const colors = (schema as any).colors || ['hsl(var(--chart-1))', 'hsl(var(--chart-2))', 'hsl(var(--chart-3))']; + const colors = (schema as any).colors || ['hsl(var(--chart-1))', 'hsl(var(--chart-2))', 'hsl(var(--chart-3))']; const newConfig: ChartConfig = {}; series.forEach((s: any, idx: number) => { - newConfig[s.dataKey] = { label: s.label || s.dataKey, color: colors[idx % colors.length] }; + newConfig[s.dataKey] = { label: s.label || s.dataKey, color: s.color || colors[idx % colors.length] }; }); config = newConfig; } return { - chartType: schema.chartType, + chartType: schema.chartType ?? spec.chartType, data: Array.isArray(schema.data) ? schema.data : [], config, xAxisKey, series, - className: schema.className + className: schema.className, + spec, }; }, [schema]); @@ -126,6 +154,14 @@ export const ChartRenderer: React.FC = ({ schema, onChartCli categoryOrder={(schema as any).categoryOrder} isAnimationActive={schema.isAnimationActive} onChartClick={onChartClick} + xAxis={props.spec.xAxis} + yAxes={props.spec.yAxes} + showLegend={props.spec.showLegend} + showDataLabels={props.spec.showDataLabels} + title={props.spec.title} + subtitle={props.spec.subtitle} + annotations={props.spec.annotations} + interaction={props.spec.interaction} /> ); diff --git a/packages/plugin-charts/src/normalizeChartSchema.test.ts b/packages/plugin-charts/src/normalizeChartSchema.test.ts new file mode 100644 index 0000000000..c7bcd5f0df --- /dev/null +++ b/packages/plugin-charts/src/normalizeChartSchema.test.ts @@ -0,0 +1,157 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * The spec→internal chart translation (objectui#2880 S1, framework#3729). + * + * Two invariants carry the whole design: + * 1. an author writing the SPEC shape gets a working chart, and + * 2. a caller already speaking the INTERNAL shape is byte-for-byte untouched + * (DashboardRenderer / ObjectView / the dataset path pass it explicitly). + */ +import { describe, it, expect } from 'vitest'; +import { normalizeChartSchema, formatterFor, domainFor } from './normalizeChartSchema'; + +describe('normalizeChartSchema — spec shape', () => { + it('resolves the ChartConfig axis + series shape', () => { + const out = normalizeChartSchema({ + type: 'line', + xAxis: { field: 'status' }, + yAxis: [{ field: 'total' }], + series: [{ name: 'total', label: 'Invoice value' }], + }); + expect(out.chartType).toBe('line'); + expect(out.xAxisKey).toBe('status'); + expect(out.series).toEqual([{ dataKey: 'total', label: 'Invoice value' }]); + }); + + it('plots from yAxis alone when no series is declared', () => { + const out = normalizeChartSchema({ xAxis: { field: 'status' }, yAxis: [{ field: 'total' }] }); + expect(out.series).toEqual([{ dataKey: 'total' }]); + }); + + it('accepts the report surface’s bare-string axes', () => { + // ReportChartSchema narrows xAxis/yAxis from objects to plain strings. + const out = normalizeChartSchema({ xAxis: 'status', yAxis: 'total' }); + expect(out.xAxisKey).toBe('status'); + expect(out.series).toEqual([{ dataKey: 'total' }]); + }); + + it('carries the axis presentation props through', () => { + const out = normalizeChartSchema({ + yAxis: [{ field: 'total', format: '$0,0.00', min: 0, max: 100, logarithmic: true, title: 'Revenue' }], + }); + expect(out.yAxes?.[0]).toMatchObject({ + field: 'total', format: '$0,0.00', min: 0, max: 100, logarithmic: true, title: 'Revenue', + }); + }); + + it('carries stack / yAxis / color on a series', () => { + const out = normalizeChartSchema({ + series: [ + { name: 'won', stack: 'deals', color: 'green' }, + { name: 'rate', yAxis: 'right', type: 'line' }, + ], + }); + expect(out.series?.[0]).toMatchObject({ dataKey: 'won', stack: 'deals', color: 'green' }); + expect(out.series?.[1]).toMatchObject({ dataKey: 'rate', yAxis: 'right', chartType: 'line' }); + }); + + it('resolves an i18n label record to a string', () => { + const out = normalizeChartSchema({ title: { 'zh-CN': '发票金额', en: 'Invoice value' } }); + expect(typeof out.title).toBe('string'); + expect(out.title).toBeTruthy(); + }); + + it('passes chrome + annotations + interaction through', () => { + const out = normalizeChartSchema({ + showLegend: false, + showDataLabels: true, + annotations: [{ type: 'line', axis: 'y', value: 100 }], + interaction: { tooltips: false, brush: true }, + }); + expect(out.showLegend).toBe(false); + expect(out.showDataLabels).toBe(true); + expect(out.annotations).toHaveLength(1); + expect(out.interaction).toEqual({ tooltips: false, brush: true }); + }); +}); + +describe('normalizeChartSchema — internal shape wins', () => { + it('keeps explicit internal props over the spec ones', () => { + // The no-migration guarantee: DashboardRenderer/ObjectView/dataset callers + // pass the internal shape and must be unaffected by this layer. + const out = normalizeChartSchema({ + chartType: 'bar', + xAxisKey: 'stage', + series: [{ dataKey: 'amount' }], + type: 'line', + xAxis: { field: 'status' }, + }); + expect(out.chartType).toBe('bar'); + expect(out.xAxisKey).toBe('stage'); + expect(out.series).toEqual([{ dataKey: 'amount' }]); + }); + + it('returns nothing for a schema with no chart binding at all', () => { + expect(normalizeChartSchema({ objectName: 'invoice' })).toEqual({}); + expect(normalizeChartSchema(null)).toEqual({}); + }); +}); + +describe('normalizeChartSchema — the `type` collision', () => { + it('does NOT read a component discriminator as a chart family', () => { + // `object-chart` is the SDUI component type, not a chart family — reading + // it as one would render a bar chart for every block on the page. + expect(normalizeChartSchema({ type: 'object-chart' }).chartType).toBeUndefined(); + }); + + it('reads `specType`, where the react-page wrapper parks an author `type`', () => { + expect(normalizeChartSchema({ type: 'object-chart', specType: 'donut' }).chartType).toBe('donut'); + }); + + it('leaves a family this renderer cannot draw unset', () => { + // `metric`/`kpi` are single-value families rendered by other components; + // mapping them onto a bar chart would draw the wrong picture silently. + expect(normalizeChartSchema({ specType: 'metric' }).chartType).toBeUndefined(); + }); +}); + +describe('formatterFor', () => { + it('formats currency', () => { + expect(formatterFor('$0,0.00')!(1234.5)).toMatch(/1,234\.50/); + }); + + it('formats percent', () => { + expect(formatterFor('0.0%')!(0.256)).toMatch(/25\.6%/); + }); + + it('groups thousands', () => { + expect(formatterFor('0,0')!(1234567)).toMatch(/1,234,567/); + }); + + it('returns undefined for an unrecognized format so the caller keeps its default', () => { + expect(formatterFor(undefined)).toBeUndefined(); + expect(formatterFor('wat')).toBeUndefined(); + }); + + it('passes non-numeric values through instead of printing NaN', () => { + expect(formatterFor('$0,0.00')!('n/a')).toBe('n/a'); + }); +}); + +describe('domainFor', () => { + it('pins both ends', () => { + expect(domainFor({ min: 0, max: 100 })).toEqual([0, 100]); + }); + + it('leaves the unspecified end automatic', () => { + expect(domainFor({ min: 0 })).toEqual([0, 'auto']); + expect(domainFor({ max: 100 })).toEqual(['auto', 100]); + }); + + it('returns undefined when neither end is set', () => { + expect(domainFor({})).toBeUndefined(); + expect(domainFor(undefined)).toBeUndefined(); + }); +}); diff --git a/packages/plugin-charts/src/normalizeChartSchema.ts b/packages/plugin-charts/src/normalizeChartSchema.ts new file mode 100644 index 0000000000..708f3b539d --- /dev/null +++ b/packages/plugin-charts/src/normalizeChartSchema.ts @@ -0,0 +1,334 @@ +/** + * 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. + */ + +/** + * The ONE place the spec's author-facing chart shape is translated into the + * renderer's internal pipeline contract (framework#3729 / objectui#2880). + * + * ## Why this exists + * + * `ChartConfigSchema` (`@objectstack/spec/ui`) is the chart protocol — axes are + * `{ field, format, min, max, logarithmic, … }` objects and series are + * `{ name, stack, yAxis, … }`. The renderer, however, grew a Recharts-flavoured + * internal shape: `chartType`, `xAxisKey` (a bare string) and + * `series[].dataKey`. Everything an author wrote in the SPEC shape reached the + * renderer and was silently dropped — precisely what ADR-0078 forbids. + * + * This module closes that gap by NORMALIZING at the boundary, rather than by + * sprinkling `??` fallbacks through the render tree. That distinction matters + * (framework PD #12): a lone normalization layer is a translation from one + * declared contract to another, whereas scattered fallbacks fossilize a second + * de-facto dialect at every read site. + * + * **Internal props win.** `DashboardRenderer`, `ObjectView` and the dataset + * path already speak the internal shape; passing `xAxisKey`/`series[].dataKey` + * explicitly keeps working byte-for-byte, so there is no migration. + * + * ## The `type` collision + * + * `ChartConfig.type` is the chart family (`'bar' | 'line' | …`). But on every + * surface that FLATTENS chart config into a component props bag — the react + * tier's injected blocks especially — `type` is already taken: it is the SDUI + * envelope's component discriminator (`{ type: 'object-chart', … }`). An author + * writing `type="bar"` would replace the discriminator and the block would not + * resolve at all. + * + * The collision is created by the flattening, so it is resolved there: the + * react-page wrapper preserves an author-supplied `type` under `specType` + * before stamping the discriminator (see `react-page.tsx`). This module reads + * it back. Surfaces where chart config is properly NESTED (a dashboard + * widget's `chartConfig`) never collide and pass `type` through untouched. + */ + +/** A chart family this renderer actually draws. */ +export type ChartFamily = + | 'bar' | 'column' | 'horizontal-bar' + | 'line' | 'area' + | 'pie' | 'donut' | 'funnel' + | 'radar' | 'scatter' + | 'treemap' | 'sankey' + | 'combo'; + +const RENDERABLE = new Set([ + 'bar', 'column', 'horizontal-bar', + 'line', 'area', + 'pie', 'donut', 'funnel', + 'radar', 'scatter', + 'treemap', 'sankey', + 'combo', +]); + +/** + * Every value the spec's `ChartTypeSchema` admits — a superset of + * {@link RENDERABLE}, because single-value families (`metric`, `kpi`, `gauge`, + * …) and tabular ones render through other components entirely. + * + * Recognition is broader than rendering on purpose: this set is what decides + * whether a bare `type` is a chart family or the SDUI envelope's component + * discriminator, and getting THAT wrong would break the block outright. + */ +const CHART_TYPES = new Set([ + ...RENDERABLE, + 'gauge', 'solid-gauge', 'metric', 'kpi', 'bullet', + 'table', 'pivot', +]); + +export type AnyRec = Record; + +/** A y-axis (or the x-axis) after normalization — spec `ChartAxis`, resolved. */ +export interface NormalizedAxis { + /** Result column this axis reads (y-axis only; the x-axis field becomes `xAxisKey`). */ + field?: string; + /** d3-style format string, e.g. `"$0,0.00"` / `"0.0%"`. */ + format?: string; + min?: number; + max?: number; + /** Default true, per the spec schema. */ + showGridLines?: boolean; + logarithmic?: boolean; + position?: 'left' | 'right' | 'top' | 'bottom'; + /** Axis title (already resolved to a plain string). */ + title?: string; +} + +export interface NormalizedSeries { + dataKey: string; + label?: string; + /** Per-series family override (combo charts). */ + chartType?: 'bar' | 'line' | 'area'; + variant?: 'current' | 'comparison' | 'primary'; + opacity?: number; + dashArray?: string; + /** Stack group id — series sharing one id stack together. */ + stack?: string; + /** Which y-axis this series binds to (dual-axis charts). */ + yAxis?: 'left' | 'right'; + color?: string; +} + +export interface NormalizedChartSchema { + chartType?: ChartFamily; + xAxisKey?: string; + series?: NormalizedSeries[]; + /** X-axis presentation config (its `field` is hoisted to `xAxisKey`). */ + xAxis?: NormalizedAxis; + /** Y-axes, in declaration order. Index 0 is the primary (left) axis. */ + yAxes?: NormalizedAxis[]; + showLegend?: boolean; + showDataLabels?: boolean; + title?: string; + subtitle?: string; + annotations?: AnyRec[]; + interaction?: AnyRec; +} + +const isRec = (v: unknown): v is AnyRec => !!v && typeof v === 'object' && !Array.isArray(v); +const str = (v: unknown): string | undefined => (typeof v === 'string' && v ? v : undefined); +const num = (v: unknown): number | undefined => (typeof v === 'number' && Number.isFinite(v) ? v : undefined); + +/** + * An i18n label may be a plain string or a `{ en, zh-CN, … }` record. Charts + * render a string; pick a reasonable one rather than `[object Object]`. + */ +function label(v: unknown): string | undefined { + const s = str(v); + if (s) return s; + if (isRec(v)) { + const first = Object.values(v).find((x) => typeof x === 'string' && x); + return first as string | undefined; + } + return undefined; +} + +function normalizeAxis(raw: unknown): NormalizedAxis | undefined { + if (!isRec(raw)) return undefined; + const out: NormalizedAxis = {}; + const field = str(raw.field); + if (field) out.field = field; + const format = str(raw.format); + if (format) out.format = format; + const min = num(raw.min); + if (min !== undefined) out.min = min; + const max = num(raw.max); + if (max !== undefined) out.max = max; + if (typeof raw.showGridLines === 'boolean') out.showGridLines = raw.showGridLines; + if (typeof raw.logarithmic === 'boolean') out.logarithmic = raw.logarithmic; + const position = str(raw.position); + if (position === 'left' || position === 'right' || position === 'top' || position === 'bottom') { + out.position = position; + } + const title = label(raw.title); + if (title) out.title = title; + return out; +} + +/** + * Merge one series entry from either shape. + * + * Internal (`dataKey`) wins over spec (`name`) so a caller that already speaks + * the internal contract is untouched. `type` (spec, a ChartType) and + * `chartType` (internal) both name the per-series family in a combo chart. + */ +function normalizeSeries(raw: unknown): NormalizedSeries | undefined { + if (!isRec(raw)) { + // A bare string is accepted as a shorthand for `{ name }` — the Tremor-ish + // `categories: ['a','b']` form ChartRenderer already adapts. + const bare = str(raw); + return bare ? { dataKey: bare } : undefined; + } + const dataKey = str(raw.dataKey) ?? str(raw.name); + if (!dataKey) return undefined; + const out: NormalizedSeries = { dataKey }; + const lbl = label(raw.label); + if (lbl) out.label = lbl; + const family = str(raw.chartType) ?? str(raw.type); + if (family === 'bar' || family === 'line' || family === 'area') out.chartType = family; + const variant = str(raw.variant); + if (variant === 'comparison' || variant === 'current' || variant === 'primary') out.variant = variant; + const opacity = num(raw.opacity); + if (opacity !== undefined) out.opacity = opacity; + const dash = str(raw.dashArray); + if (dash) out.dashArray = dash; + const stack = str(raw.stack); + if (stack) out.stack = stack; + const yAxis = str(raw.yAxis); + if (yAxis === 'left' || yAxis === 'right') out.yAxis = yAxis; + const color = str(raw.color); + if (color) out.color = color; + return out; +} + +/** + * Translate a chart schema — spec shape, internal shape, or a mix — into the + * renderer's internal contract. Only keys that resolve to something are + * present on the result, so callers can spread it over their own defaults. + */ +export function normalizeChartSchema(schema: unknown): NormalizedChartSchema { + if (!isRec(schema)) return {}; + const out: NormalizedChartSchema = {}; + + // ── chart family ──────────────────────────────────────────────────────── + // `chartType` (internal) → `specType` (an author `type` rescued from the + // envelope collision) → `type` (only when it is unambiguously a chart family + // and not a component discriminator). + const rawType = str(schema.type); + const chartType = + str(schema.chartType) ?? + str(schema.specType) ?? + (rawType && CHART_TYPES.has(rawType) ? rawType : undefined); + // A family this renderer does not draw (`metric`, `table`, …) is left unset + // rather than mapped onto a bar chart — the caller's own default is a more + // honest answer than silently drawing the wrong picture. + if (chartType && RENDERABLE.has(chartType)) out.chartType = chartType as ChartFamily; + + // ── axes ──────────────────────────────────────────────────────────────── + // Spec `xAxis` is an object; the report surface narrows it to a bare string. + // Both mean "the column on the category axis". + const xAxisRaw = schema.xAxis; + const xAxisSpec = normalizeAxis(xAxisRaw); + if (xAxisSpec && (xAxisSpec.format || xAxisSpec.title || xAxisSpec.showGridLines !== undefined)) { + out.xAxis = xAxisSpec; + } + const xAxisKey = str(schema.xAxisKey) ?? xAxisSpec?.field ?? str(xAxisRaw); + if (xAxisKey) out.xAxisKey = xAxisKey; + + const yAxes = (Array.isArray(schema.yAxis) ? schema.yAxis : schema.yAxis !== undefined ? [schema.yAxis] : []) + .map((a: unknown) => normalizeAxis(a) ?? (str(a) ? { field: str(a) } : undefined)) + .filter((a): a is NormalizedAxis => !!a); + if (yAxes.length) out.yAxes = yAxes; + + // ── series ────────────────────────────────────────────────────────────── + const rawSeries = Array.isArray(schema.series) + ? schema.series + : Array.isArray(schema.categories) + ? schema.categories + : undefined; + let series = rawSeries?.map(normalizeSeries).filter((s): s is NormalizedSeries => !!s); + // No series at all: the y-axes name the plotted columns, so a chart written + // purely in spec shape (`yAxis: [{ field: 'total' }]`) still plots. + if (!series?.length) { + const fromAxes = yAxes + .filter((a) => a.field) + .map((a, i) => ({ + dataKey: a.field!, + ...(a.title ? { label: a.title } : {}), + ...(i > 0 || a.position === 'right' ? { yAxis: 'right' as const } : {}), + })); + if (fromAxes.length) series = fromAxes; + } + if (series?.length) out.series = series; + + // ── chrome ────────────────────────────────────────────────────────────── + if (typeof schema.showLegend === 'boolean') out.showLegend = schema.showLegend; + if (typeof schema.showDataLabels === 'boolean') out.showDataLabels = schema.showDataLabels; + const title = label(schema.title); + if (title) out.title = title; + const subtitle = label(schema.subtitle); + if (subtitle) out.subtitle = subtitle; + if (Array.isArray(schema.annotations) && schema.annotations.length) { + out.annotations = schema.annotations.filter(isRec); + } + if (isRec(schema.interaction)) out.interaction = schema.interaction; + + return out; +} + +/** + * Build a tick formatter from a spec `ChartAxis.format` string. + * + * The spec documents "d3-format or similar" (`"$0,0.00"`, `"0.0%"`). Rather + * than pull in d3-format for a handful of patterns, this reads the shape of + * the string — currency prefix, percent suffix, thousands separator, decimal + * places — and drives `Intl.NumberFormat`, which is already loaded and + * locale-aware. An unrecognized format returns `undefined`, so the caller + * keeps its own default formatting rather than rendering something wrong. + */ +export function formatterFor(format: string | undefined): ((value: any) => string) | undefined { + if (!format) return undefined; + const isPercent = format.includes('%'); + const currencyMatch = /^([$£€¥₹])/.exec(format); + const grouped = format.includes(','); + const decimals = /\.(0+)/.exec(format)?.[1].length ?? 0; + const CURRENCY_BY_SYMBOL: Record = { + $: 'USD', '£': 'GBP', '€': 'EUR', '¥': 'CNY', '₹': 'INR', + }; + // Nothing recognizable to act on — don't guess. + if (!isPercent && !currencyMatch && !grouped && decimals === 0) return undefined; + + const opts: Intl.NumberFormatOptions = { + minimumFractionDigits: decimals, + maximumFractionDigits: decimals, + useGrouping: grouped, + }; + if (isPercent) opts.style = 'percent'; + else if (currencyMatch) { + opts.style = 'currency'; + opts.currency = CURRENCY_BY_SYMBOL[currencyMatch[1]] ?? 'USD'; + } + return (value: any) => { + const n = typeof value === 'number' ? value : Number(value); + if (!Number.isFinite(n)) return value == null ? '' : String(value); + try { + return new Intl.NumberFormat(undefined, opts).format(n); + } catch { + return String(n); + } + }; +} + +/** + * Recharts `domain` for an axis, or `undefined` to keep the auto domain. + * A half-open range (only `min`, only `max`) pins that end and leaves the + * other automatic. + */ +export function domainFor(axis: NormalizedAxis | undefined): [any, any] | undefined { + if (!axis) return undefined; + const { min, max } = axis; + if (min === undefined && max === undefined) return undefined; + return [min ?? 'auto', max ?? 'auto']; +}