Skip to content

Commit aa88056

Browse files
authored
feat(charts): honor ChartAxis.stepSize, ChartConfig.description and .height (framework#3752) (#2888)
The tail of the declared-≠-delivered sweep from #2880 / framework#3729. Three ChartConfig props reached the renderer and did nothing. stepSize: Recharts has no "a tick every N units" prop — tickCount is a hint it may ignore and interval is for categorical axes — so honoring a step means handing it the tick array outright. ticksFor builds it from the axis's own min/max where declared and from the plotted values otherwise. A data-derived max rounds UP to the next step (otherwise the topmost bar sits above the last gridline and the axis reads as truncated); an explicit max clamps instead, since a tick outside a pinned domain would be drawn outside the plot. The step-count division carries a small epsilon because 0.5/0.1 is 5.000000000000001 one way and 4.999999999999999 the other, and a bare floor() drops a whole tick in the second case. More than 200 ticks is refused rather than rendered. description: the accessibility description. A chart is a picture to a screen reader; the container now carries role="img" + aria-label. Without a description it stays an ordinary div, because stamping role="img" on an unlabelled graphic is worse than leaving one a screen reader can skip. height: was read only by the legacy ChartBarRenderer, not by the advanced path that draws every real chart. Now an inline style on the chart container, which beats its default h-[350px] class. height and description ride on the shared container props, so they apply to all eight chart families rather than one branch. ChartInteraction.zoom and .clickAction are removed from the schema in the framework companion rather than implemented here — brush already narrows a range, and a segment click already has two working owners (drillDown, onSegmentClick).
1 parent c6aaed8 commit aa88056

6 files changed

Lines changed: 230 additions & 8 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
"@object-ui/plugin-charts": minor
3+
---
4+
5+
feat(charts): honor `ChartAxis.stepSize`, `ChartConfig.description` and `ChartConfig.height` (framework#3752)
6+
7+
The tail of the declared-≠-delivered sweep from framework#3729 / #2880. Three
8+
`ChartConfig` props reached the renderer and did nothing:
9+
10+
- **`ChartAxis.stepSize`** — Recharts has no "a tick every N units" prop
11+
(`tickCount` is a hint it may ignore, `interval` is for categorical axes), so
12+
honoring a step means handing it the tick array outright. `ticksFor` builds it
13+
from the axis's own `min`/`max` where declared and from the plotted values
14+
otherwise, so a step works with or without a pinned domain. A data-derived max
15+
rounds UP to the next step (otherwise the topmost bar sits above the last
16+
gridline and the axis reads as truncated); an explicit `max` clamps instead,
17+
since a tick outside a pinned domain would be drawn outside the plot. A step
18+
that would produce more than 200 ticks is refused rather than rendered — that
19+
is a wrong config, and drawing it would hang the page instead of surfacing the
20+
mistake.
21+
- **`ChartConfig.description`** — the accessibility description. A chart is a
22+
picture to a screen reader; the container now carries `role="img"` +
23+
`aria-label`. Without a description it stays an ordinary div, because
24+
stamping `role="img"` on an *unlabelled* graphic is worse than leaving one a
25+
screen reader can skip.
26+
- **`ChartConfig.height`** — was read only by the legacy `ChartBarRenderer`, not
27+
by the advanced path that draws every real chart. Now applied to the chart
28+
container as an inline style, which beats its default `h-[350px]` class.
29+
30+
`height` and `description` ride on the shared container props, so they apply to
31+
all eight chart families rather than one branch.

packages/plugin-charts/src/AdvancedChartImpl.specConfig.test.tsx

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,36 @@ describe('AdvancedChartImpl — spec annotations', () => {
151151
});
152152
});
153153

154+
describe('AdvancedChartImpl — spec container props (#3752)', () => {
155+
it('announces the accessibility description', () => {
156+
const { container } = render(<AdvancedChartImpl {...base} description="Invoice value by status" />);
157+
const el = container.querySelector('[data-slot="chart"]');
158+
expect(el?.getAttribute('role')).toBe('img');
159+
expect(el?.getAttribute('aria-label')).toBe('Invoice value by status');
160+
});
161+
162+
it('leaves the container unlabelled when no description is declared', () => {
163+
// No description is not the same as an empty one — don't stamp role="img"
164+
// on an unlabelled graphic, which is worse for a screen reader than a
165+
// plain div it can skip.
166+
const { container } = render(<AdvancedChartImpl {...base} />);
167+
expect(container.querySelector('[data-slot="chart"]')?.getAttribute('role')).toBeNull();
168+
});
169+
170+
it('applies an explicit height over the container default', () => {
171+
const { container } = render(<AdvancedChartImpl {...base} height={420} />);
172+
expect((container.querySelector('[data-slot="chart"]') as HTMLElement)?.style.height).toBe('420px');
173+
});
174+
175+
it('lays y ticks at the declared stepSize', () => {
176+
const { container } = render(
177+
<AdvancedChartImpl {...base} yAxes={[{ field: 'total', min: 0, max: 120, stepSize: 40 }]} />,
178+
);
179+
const texts = Array.from(container.querySelectorAll('text')).map((t) => t.textContent ?? '');
180+
for (const tick of ['0', '40', '80', '120']) expect(texts).toContain(tick);
181+
});
182+
});
183+
154184
describe('AdvancedChartImpl — spec interaction', () => {
155185
it('adds the brush when interaction.brush is on', () => {
156186
const { container } = render(<AdvancedChartImpl {...base} interaction={{ brush: true }} />);

packages/plugin-charts/src/AdvancedChartImpl.tsx

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import {
4747
ChartConfig
4848
} from './ChartContainerImpl';
4949
import { mapScatterClick, mapTreemapClick, mapSankeyClick } from './chartDrillEvents';
50-
import { formatterFor, domainFor, type NormalizedAxis, type NormalizedSeries } from './normalizeChartSchema';
50+
import { formatterFor, domainFor, ticksFor, type NormalizedAxis, type NormalizedSeries } from './normalizeChartSchema';
5151
import { buildCategoryRank } from '@object-ui/core';
5252

5353
// Default color fallback for chart series
@@ -160,6 +160,14 @@ export interface AdvancedChartImplProps {
160160
/** Spec `ChartConfig.title` / `.subtitle`, rendered above the plot. */
161161
title?: string;
162162
subtitle?: string;
163+
/**
164+
* Spec `ChartConfig.description` — the accessibility description. A chart is
165+
* a picture to a screen reader; without this it announces as an unlabelled
166+
* graphic, so the container carries it as `role="img"` + `aria-label`.
167+
*/
168+
description?: string;
169+
/** Spec `ChartConfig.height` — fixed plot height in pixels. */
170+
height?: number;
163171
/** Spec `ChartConfig.annotations` — reference lines / bands. */
164172
annotations?: Array<Record<string, any>>;
165173
/** Spec `ChartConfig.interaction` — `tooltips`, `brush`. */
@@ -238,6 +246,8 @@ export default function AdvancedChartImpl({
238246
showDataLabels,
239247
title,
240248
subtitle,
249+
description,
250+
height,
241251
annotations,
242252
interaction,
243253
}: AdvancedChartImplProps) {
@@ -252,7 +262,15 @@ export default function AdvancedChartImpl({
252262
// tell ChartContainer to skip its settle re-mount — avoids a needless 1-frame
253263
// reflow on the dashboard's first paint (#2756). Animated callers keep the
254264
// heal; this object is empty for them, leaving their markup unchanged.
255-
const containerProps = isAnimationActive === false ? { disableSettleRemount: true } : {};
265+
// Everything every ChartContainer call site needs, so a new container-level
266+
// spec prop lands on all eight chart families at once instead of one branch.
267+
const containerProps = {
268+
...(isAnimationActive === false ? { disableSettleRemount: true as const } : {}),
269+
// An explicit height beats the container's default `h-[350px]` class
270+
// because an inline style wins over a utility class.
271+
...(height ? { style: { height } } : {}),
272+
...(description ? { role: 'img' as const, 'aria-label': description } : {}),
273+
};
256274
const [isMobile, setIsMobile] = React.useState(false);
257275

258276
// Recharts' top-level onClick payload: { activeLabel, activePayload, ... }
@@ -434,19 +452,40 @@ export default function AdvancedChartImpl({
434452
[secondaryY?.format, formatYTick],
435453
);
436454

437-
/** Recharts props derived from one spec y-axis (domain / scale / label). */
438-
const yAxisSpecProps = React.useCallback((axis: NormalizedAxis | undefined) => {
455+
/**
456+
* Every number plotted on one side of a dual axis (or on the only axis) —
457+
* the range `stepSize` lays its ticks over.
458+
*/
459+
const axisValues = React.useCallback((side: 'left' | 'right') => {
460+
const keys = series
461+
.filter((s: any) => (hasDualAxis ? (s.yAxis === 'right' ? 'right' : 'left') === side : side === 'left'))
462+
.map((s: any) => s.dataKey);
463+
const out: number[] = [];
464+
for (const row of data) {
465+
for (const k of keys) {
466+
const n = Number(row?.[k]);
467+
if (Number.isFinite(n)) out.push(n);
468+
}
469+
}
470+
return out;
471+
// eslint-disable-next-line react-hooks/exhaustive-deps
472+
}, [data, series, hasDualAxis]);
473+
474+
/** Recharts props derived from one spec y-axis (domain / scale / ticks / label). */
475+
const yAxisSpecProps = React.useCallback((axis: NormalizedAxis | undefined, side: 'left' | 'right' = 'left') => {
439476
if (!axis) return {};
440477
const domain = domainFor(axis);
478+
const ticks = ticksFor(axis, axisValues(side));
441479
return {
480+
...(ticks ? { ticks } : {}),
442481
...(domain ? { domain } : {}),
443482
// `allowDataOverflow` is what makes an explicit domain actually clip
444483
// rather than being silently widened to fit the data.
445484
...(domain ? { allowDataOverflow: true } : {}),
446485
...(axis.logarithmic ? { scale: 'log' as const, domain: domain ?? ([1, 'auto'] as any) } : {}),
447486
...(axis.title ? { label: { value: axis.title, angle: -90, position: 'insideLeft' as const } } : {}),
448487
};
449-
}, []);
488+
}, [axisValues]);
450489

451490
// `showGridLines` is per-axis in the spec; the renderer draws one grid, so
452491
// an explicit `false` on EITHER axis turns off that axis's lines.
@@ -785,7 +824,7 @@ export default function AdvancedChartImpl({
785824
<CartesianGrid {...gridProps} />
786825
<XAxis dataKey={xAxisKey} {...xAxisCommonProps} />
787826
<YAxis yAxisId="left" tickLine={false} axisLine={false} tickFormatter={yTickFormatter} width={48} {...yAxisSpecProps(primaryY)} />
788-
<YAxis yAxisId="right" orientation="right" tickLine={false} axisLine={false} tickFormatter={y2TickFormatter} width={48} {...yAxisSpecProps(secondaryY)} />
827+
<YAxis yAxisId="right" orientation="right" tickLine={false} axisLine={false} tickFormatter={y2TickFormatter} width={48} {...yAxisSpecProps(secondaryY, 'right')} />
789828
{tooltipsEnabled ? <ChartTooltip content={<ChartTooltipContent />} /> : null}
790829
{legendVisible ? (
791830
<ChartLegend
@@ -902,7 +941,7 @@ export default function AdvancedChartImpl({
902941
axisLine={false}
903942
tickFormatter={y2TickFormatter}
904943
width={48}
905-
{...yAxisSpecProps(secondaryY)}
944+
{...yAxisSpecProps(secondaryY, 'right')}
906945
/>
907946
) : null}
908947
</>

packages/plugin-charts/src/ChartRenderer.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ export interface ChartRendererProps {
6161
showDataLabels?: boolean;
6262
title?: unknown;
6363
subtitle?: unknown;
64+
description?: unknown;
65+
height?: number;
6466
annotations?: Array<Record<string, any>>;
6567
interaction?: Record<string, any>;
6668
/** An author `type` rescued from the SDUI envelope's discriminator
@@ -160,6 +162,8 @@ export const ChartRenderer: React.FC<ChartRendererProps> = ({ schema, onChartCli
160162
showDataLabels={props.spec.showDataLabels}
161163
title={props.spec.title}
162164
subtitle={props.spec.subtitle}
165+
description={props.spec.description}
166+
height={props.spec.height ?? schema.height}
163167
annotations={props.spec.annotations}
164168
interaction={props.spec.interaction}
165169
/>

packages/plugin-charts/src/normalizeChartSchema.test.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* (DashboardRenderer / ObjectView / the dataset path pass it explicitly).
1111
*/
1212
import { describe, it, expect } from 'vitest';
13-
import { normalizeChartSchema, formatterFor, domainFor } from './normalizeChartSchema';
13+
import { normalizeChartSchema, formatterFor, domainFor, ticksFor } from './normalizeChartSchema';
1414

1515
describe('normalizeChartSchema — spec shape', () => {
1616
it('resolves the ChartConfig axis + series shape', () => {
@@ -140,6 +140,59 @@ describe('formatterFor', () => {
140140
});
141141
});
142142

143+
describe('normalizeChartSchema — container-level props (#3752)', () => {
144+
it('carries description and height', () => {
145+
const out = normalizeChartSchema({ description: 'Invoice value by status', height: 320 });
146+
expect(out.description).toBe('Invoice value by status');
147+
expect(out.height).toBe(320);
148+
});
149+
150+
it('ignores a non-positive height rather than collapsing the chart', () => {
151+
expect(normalizeChartSchema({ height: 0 }).height).toBeUndefined();
152+
expect(normalizeChartSchema({ height: -10 }).height).toBeUndefined();
153+
});
154+
155+
it('carries stepSize on an axis', () => {
156+
expect(normalizeChartSchema({ yAxis: [{ field: 'total', stepSize: 25 }] }).yAxes?.[0].stepSize).toBe(25);
157+
});
158+
159+
it('ignores a non-positive stepSize', () => {
160+
expect(normalizeChartSchema({ yAxis: [{ field: 'total', stepSize: 0 }] }).yAxes?.[0].stepSize).toBeUndefined();
161+
});
162+
});
163+
164+
describe('ticksFor', () => {
165+
it('lays ticks over the data range at the declared step', () => {
166+
expect(ticksFor({ stepSize: 50 }, [10, 120])).toEqual([0, 50, 100, 150]);
167+
});
168+
169+
it('honours an explicit domain over the data', () => {
170+
expect(ticksFor({ min: 100, max: 300, stepSize: 100 }, [10, 20])).toEqual([100, 200, 300]);
171+
});
172+
173+
it('reaches an explicit max the step would otherwise overshoot', () => {
174+
expect(ticksFor({ min: 0, max: 25, stepSize: 10 }, [])).toEqual([0, 10, 20, 25]);
175+
});
176+
177+
it('does not drift on a fractional step', () => {
178+
expect(ticksFor({ min: 0, max: 0.5, stepSize: 0.1 }, [])).toEqual([0, 0.1, 0.2, 0.3, 0.4, 0.5]);
179+
});
180+
181+
it('returns undefined with no stepSize — Recharts keeps its automatic ticks', () => {
182+
expect(ticksFor({ min: 0, max: 100 }, [])).toBeUndefined();
183+
expect(ticksFor(undefined, [])).toBeUndefined();
184+
});
185+
186+
it('refuses an absurd tick count instead of hanging the page', () => {
187+
// 100_000 / 0.5 ticks is a wrong config, not a chart.
188+
expect(ticksFor({ stepSize: 0.5 }, [0, 100_000])).toBeUndefined();
189+
});
190+
191+
it('returns undefined when there is nothing to measure', () => {
192+
expect(ticksFor({ stepSize: 10 }, [])).toBeUndefined();
193+
});
194+
});
195+
143196
describe('domainFor', () => {
144197
it('pins both ends', () => {
145198
expect(domainFor({ min: 0, max: 100 })).toEqual([0, 100]);

packages/plugin-charts/src/normalizeChartSchema.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ export interface NormalizedAxis {
8888
format?: string;
8989
min?: number;
9090
max?: number;
91+
/** Distance between ticks on this axis. */
92+
stepSize?: number;
9193
/** Default true, per the spec schema. */
9294
showGridLines?: boolean;
9395
logarithmic?: boolean;
@@ -123,6 +125,10 @@ export interface NormalizedChartSchema {
123125
showDataLabels?: boolean;
124126
title?: string;
125127
subtitle?: string;
128+
/** Accessibility description — announced to screen readers. */
129+
description?: string;
130+
/** Fixed plot height in pixels. */
131+
height?: number;
126132
annotations?: AnyRec[];
127133
interaction?: AnyRec;
128134
}
@@ -156,6 +162,8 @@ function normalizeAxis(raw: unknown): NormalizedAxis | undefined {
156162
if (min !== undefined) out.min = min;
157163
const max = num(raw.max);
158164
if (max !== undefined) out.max = max;
165+
const stepSize = num(raw.stepSize);
166+
if (stepSize !== undefined && stepSize > 0) out.stepSize = stepSize;
159167
if (typeof raw.showGridLines === 'boolean') out.showGridLines = raw.showGridLines;
160168
if (typeof raw.logarithmic === 'boolean') out.logarithmic = raw.logarithmic;
161169
const position = str(raw.position);
@@ -270,6 +278,10 @@ export function normalizeChartSchema(schema: unknown): NormalizedChartSchema {
270278
if (title) out.title = title;
271279
const subtitle = label(schema.subtitle);
272280
if (subtitle) out.subtitle = subtitle;
281+
const description = label(schema.description);
282+
if (description) out.description = description;
283+
const height = num(schema.height);
284+
if (height !== undefined && height > 0) out.height = height;
273285
if (Array.isArray(schema.annotations) && schema.annotations.length) {
274286
out.annotations = schema.annotations.filter(isRec);
275287
}
@@ -321,6 +333,59 @@ export function formatterFor(format: string | undefined): ((value: any) => strin
321333
};
322334
}
323335

336+
/**
337+
* Explicit tick positions for an axis that declares a `stepSize`, or
338+
* `undefined` to keep Recharts' automatic ticks.
339+
*
340+
* Recharts has no "every N units" prop — `tickCount` is a hint it may ignore
341+
* and `interval` is for categorical axes — so honoring `stepSize` means
342+
* handing it the tick array outright. The range comes from the axis's own
343+
* `min`/`max` where declared and from the plotted values otherwise, so a step
344+
* works with or without a pinned domain.
345+
*
346+
* `values` should be every number plotted on this axis. An empty range, a
347+
* non-finite one, or a step that would produce an absurd number of ticks
348+
* (>`MAX_TICKS`) yields `undefined` — a 10,000-tick axis is a wrong config,
349+
* and drawing it would hang the page rather than report the mistake.
350+
*/
351+
const MAX_TICKS = 200;
352+
353+
export function ticksFor(axis: NormalizedAxis | undefined, values: number[]): number[] | undefined {
354+
const step = axis?.stepSize;
355+
if (!step || step <= 0) return undefined;
356+
357+
const finite = values.filter((v) => Number.isFinite(v));
358+
const dataMin = finite.length ? Math.min(...finite) : undefined;
359+
const dataMax = finite.length ? Math.max(...finite) : undefined;
360+
// A value axis conventionally starts at zero unless told otherwise, which is
361+
// also what Recharts' own auto domain does for bars/areas.
362+
const lo = axis.min ?? Math.min(0, dataMin ?? 0);
363+
const hi = axis.max ?? dataMax;
364+
if (hi === undefined || !Number.isFinite(lo) || !Number.isFinite(hi) || hi < lo) return undefined;
365+
366+
const start = Math.floor(lo / step) * step;
367+
// An explicit `max` pins the domain, so the last tick must not overshoot it
368+
// (Recharts would place a tick outside the plot). A data-derived max is not
369+
// pinned, so round UP to the next step — otherwise the topmost value sits
370+
// above the last gridline and the axis reads as truncated.
371+
const end = axis.max !== undefined ? axis.max : Math.ceil(hi / step) * step;
372+
// `(end - start) / step` is exact in decimal but not in binary: 0.5 / 0.1 is
373+
// 5.000000000000001 one way and 4.999999999999999 the other, and a bare
374+
// floor() drops a whole tick in the second case.
375+
const count = Math.floor((end - start) / step + 1e-9) + 1;
376+
if (count < 1 || count > MAX_TICKS) return undefined;
377+
378+
const out: number[] = [];
379+
for (let i = 0; i < count; i++) {
380+
// Re-derive from the index rather than accumulating, so a fractional step
381+
// (0.1) does not drift into 0.30000000000000004 territory across the axis.
382+
out.push(Number((start + i * step).toPrecision(12)));
383+
}
384+
// Make sure an explicit max is actually reachable as a tick.
385+
if (axis.max !== undefined && out[out.length - 1] < axis.max) out.push(axis.max);
386+
return out;
387+
}
388+
324389
/**
325390
* Recharts `domain` for an axis, or `undefined` to keep the auto domain.
326391
* A half-open range (only `min`, only `max`) pins that end and leaves the

0 commit comments

Comments
 (0)