From 74d724a534217fc092484c460d58d3a0f61ba78f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 04:10:23 +0000 Subject: [PATCH 1/4] fix(plugin-charts): data labels print in their value axis's units and keep headroom above the tallest mark Claude-Session: https://claude.ai/code/session_01JQwxbY75sPQWcfUg8cZQhg Co-authored-by: Claude --- ...dvancedChartImpl.dataLabels-27372.test.tsx | 160 ++++++++++++++++++ .../plugin-charts/src/AdvancedChartImpl.tsx | 60 +++++-- 2 files changed, 209 insertions(+), 11 deletions(-) create mode 100644 ui/packages/plugin-charts/src/AdvancedChartImpl.dataLabels-27372.test.tsx diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.dataLabels-27372.test.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.dataLabels-27372.test.tsx new file mode 100644 index 00000000000..92775f09eb6 --- /dev/null +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.dataLabels-27372.test.tsx @@ -0,0 +1,160 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * objectstack#27372 — a chart's data labels (`showDataLabels`) read in the + * units of their value axis, and the tallest mark keeps its label on the + * chart surface. + * + * Two defects, one label: + * - The value axis falls back to the compact formatter (`400K`) when no + * `yAxis.format` is authored; the data label had no fallback and printed + * the raw number (`174000` beside a `400K` tick). + * - The label sits above its mark. With recharts' default 5px top margin, a + * mark on the plot's top edge (the domain's nice max) had its label drawn + * wholly above the chart surface. + * + * happy-dom paints no text, but given a fixed box recharts computes every mark + * and label position arithmetically (the same property + * `AdvancedChartImpl.pointAxisEdgeLabels-13390.test.tsx` relies on), so these + * pins read the label's text and its `y` from the DOM. + */ +import React from 'react'; +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; + +const WIDTH = 520; +const HEIGHT = 300; + +vi.mock('recharts', async () => { + const actual = await vi.importActual('recharts'); + return { + ...actual, + ResponsiveContainer: ({ children }: { children: React.ReactElement<{ width: number; height: number }> }) => + React.cloneElement(children, { width: WIDTH, height: HEIGHT }), + }; +}); + +import AdvancedChartImpl from './AdvancedChartImpl'; + +afterEach(cleanup); + +/** The card's shape: the tallest value lands on a nice top tick (400K). */ +const PIPELINE = [ + { stage: 'Prospecting', amount: 174000, won: 12 }, + { stage: 'Qualification', amount: 400000, won: 30 }, + { stage: 'Proposal', amount: 251000, won: 18 }, + { stage: 'Negotiation', amount: 92500, won: 7 }, +]; + +/** The font size the chart prints data labels in. */ +const LABEL_FONT_PX = 11; + +type Family = NonNullable['chartType']>; + +function draw(chartType: Family, extra: Record = {}) { + const { container } = render( + , + ); + return container; +} + +/** Each data label: its text and the `y` its baseline is drawn at. */ +function labelsOf(container: HTMLElement) { + const texts = [...container.querySelectorAll('.recharts-label-list text')]; + expect(texts.length, 'the chart drew no data labels, so this measures nothing').toBeGreaterThan(0); + return texts.map((t) => ({ text: (t.textContent ?? '').trim(), y: Number(t.getAttribute('y')) })); +} + +/** The y-axis tick texts, so a label can be read against its own axis. */ +function yTicksOf(container: HTMLElement, axis = '.recharts-yAxis') { + return [...container.querySelectorAll(`${axis} .recharts-cartesian-axis-tick-value`)].map((t) => + (t.textContent ?? '').trim(), + ); +} + +describe('objectstack#27372 — a data label reads in its value axis’s units', () => { + for (const chartType of ['bar', 'line', 'area'] as const) { + it(`prints the compact default beside a compact ${chartType} axis when no format is authored`, () => { + const container = draw(chartType); + // The axis is the reference: it must itself be compact, or the label + // matching it would prove nothing about units. + expect(yTicksOf(container)).toContain('400K'); + expect(labelsOf(container).map((l) => l.text)).toEqual(['174K', '400K', '251K', '92.5K']); + }); + } + + it('lets an authored axis format win over the compact default', () => { + const container = draw('bar', { yAxes: [{ field: 'amount', format: '$0,0' }] }); + expect(labelsOf(container).map((l) => l.text)).toEqual(['$174,000', '$400,000', '$251,000', '$92,500']); + }); + + it('prints each series in the units of the axis it is bound to on a dual-axis chart', () => { + const container = draw('bar', { + series: [ + { dataKey: 'amount', label: 'Amount' }, + { dataKey: 'won', label: 'Won', yAxis: 'right' }, + ], + yAxes: [{ field: 'amount' }, { field: 'won', position: 'right', format: '0.0' }], + }); + expect(labelsOf(container).map((l) => l.text)).toEqual([ + '174K', '400K', '251K', '92.5K', + '12.0', '30.0', '18.0', '7.0', + ]); + }); + + it('prints a combo series in the units of the axis it is drawn against', () => { + const container = draw('combo', { + series: [ + { dataKey: 'amount', label: 'Amount', chartType: 'bar' }, + { dataKey: 'won', label: 'Won', chartType: 'line', yAxis: 'right' }, + ], + yAxes: [{ field: 'amount' }, { field: 'won', position: 'right', format: '0.0' }], + }); + expect(labelsOf(container).map((l) => l.text)).toEqual([ + '174K', '400K', '251K', '92.5K', + '12.0', '30.0', '18.0', '7.0', + ]); + }); +}); + +describe('objectstack#27372 — the tallest mark keeps its data label on the chart surface', () => { + for (const chartType of ['bar', 'line', 'area', 'combo'] as const) { + it(`draws the top label of a ${chartType} chart inside the surface`, () => { + const container = draw(chartType); + const labels = labelsOf(container); + const top = labels.reduce((a, b) => (b.y < a.y ? b : a)); + expect(top.text).toBe('400K'); + // The label's baseline is at `y`; its glyphs rise at most one font size + // above it. Below that, the label is (partly) off the top of the svg. + expect(top.y - LABEL_FONT_PX, `"${top.text}" is drawn above the chart surface`).toBeGreaterThanOrEqual(0); + }); + } + + it('keeps recharts’ default top margin when data labels are off', () => { + const { container } = render( + , + ); + const plotTop = Number(container.querySelector('defs clipPath rect')?.getAttribute('y')); + expect(plotTop).toBe(5); + }); +}); diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx index 189fd87fd27..dc1e8457ef0 100644 --- a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -482,6 +482,36 @@ function estimateLabelWidthPx(label: string): number { /** recharts' default chart margin, left plus right (5px each side). */ const CHART_MARGIN_X_PX = 10; +/** The font size `showDataLabels` prints each point's value in, in px. */ +const DATA_LABEL_FONT_PX = 11; +/** recharts' `LabelList` default `offset`: the gap between a mark and its label. */ +const DATA_LABEL_OFFSET_PX = 5; + +/** + * The chart margin a vertical cartesian chart (bar, line, area, combo) takes + * while `showDataLabels` is on (objectstack#27372). + * + * A data label sits ABOVE its mark: its baseline is `DATA_LABEL_OFFSET_PX` + * over the mark's top. recharts' default margin leaves 5px between the plot's + * top edge and the end of the chart surface, and the value axis's nice domain + * routinely puts the tallest mark exactly on that edge (a 400,000 bar under a + * 400K top tick). Its label was then drawn wholly outside the surface: the + * tallest bar, the one a reader looks at first, was the one with no value. + * + * The top margin is that 5px plus the label's own height (offset and font), + * so a mark ON the top edge still has its label inside the surface. A margin + * rather than a padded domain: an author's `yAxis.max` pins the domain and + * must keep doing so, and the margin holds whatever the domain is. The other + * three sides stay recharts' default 5px, which `CHART_MARGIN_X_PX` counts on. + * Horizontal bars are left alone: their value runs across the plot. + */ +const DATA_LABEL_CHART_MARGIN = { + top: 5 + DATA_LABEL_OFFSET_PX + DATA_LABEL_FONT_PX, + right: 5, + bottom: 5, + left: 5, +} as const; + /** * The inset, in px, a line or area chart's categorical x axis keeps at BOTH * ends of the plot (objectui#13390). @@ -2128,10 +2158,6 @@ function AdvancedChartImplInner({ yAxisPositionNotes(yPlacement, categoriesRunDown), yAxisUndrawnNotes(yPlacement, yAxes, categoriesRunDown), ); - /** The entry a series' marks are measured against — for its value labels. */ - const axisOfSeries = (s: { yAxis?: string }): NormalizedAxis | undefined => - hasDualAxis ? (s.yAxis === 'right' ? rightY : leftY) : primaryY; - const xTickFormatter = React.useMemo( () => formatterFor(xAxisSpec?.format, displayLocale) ?? formatTick, [xAxisSpec?.format, displayLocale, formatTick], @@ -2196,6 +2222,12 @@ function AdvancedChartImplInner({ () => formatterFor(rightY?.format, displayLocale) ?? formatYTick, [rightY?.format, displayLocale, formatYTick], ); + /** + * The tick formatter of the value axis a series' marks are measured + * against — the one its value labels print with (objectstack#27372). + */ + const valueTickFormatterOf = (s: { yAxis?: string }): ((v: any) => string) => + hasDualAxis ? (s.yAxis === 'right' ? rightYTickFormatter : leftYTickFormatter) : yTickFormatter; /** * Every number plotted in one slot of a dual axis (or on the only axis, @@ -2317,10 +2349,16 @@ function AdvancedChartImplInner({ ? : null; - /** `showDataLabels` prints each point's value on the mark itself. */ - const dataLabel = (formatter?: (v: any) => string) => + /** + * `showDataLabels` prints each point's value on the mark itself, in the + * units of the value axis the mark is measured against (objectstack#27372): + * the caller passes that axis's own tick formatter, so an authored + * `yAxis.format` wins and an axis without one gives its label the compact + * default (`174K` beside a `400K` tick, never a raw `174000`). + */ + const dataLabel = (formatter: (v: any) => string) => showDataLabels - ? + ? : null; // Shared X-axis props for time/categorical axes, in two branches. @@ -3034,7 +3072,7 @@ function AdvancedChartImplInner({ happens at chart level because that is the only place the CATEGORY is known — a line/area item handler is handed the curve's props and no datum. */} - + {/* A combo always draws both value axes (an authored combo binds an @@ -3072,7 +3110,7 @@ function AdvancedChartImplInner({ : (seriesType === 'bar' ? 'left' : 'right'); const pres = seriesStyle(s, seriesType as any); const stackProps = s.stack ? { stackId: String(s.stack) } : {}; - const valueFormatter = formatterFor((yAxisId === 'right' ? rightY : leftY)?.format, displayLocale); + const valueFormatter = yAxisId === 'right' ? rightYTickFormatter : leftYTickFormatter; if (seriesType === 'line') { return ( @@ -3122,7 +3160,7 @@ function AdvancedChartImplInner({ - + {gradColors.map((c) => ( @@ -3263,7 +3301,7 @@ function AdvancedChartImplInner({ // 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(axisOfSeries(s)?.format, displayLocale); + const valueFormatter = valueTickFormatterOf(s); if (chartType === 'bar' || chartType === 'horizontal-bar') { // One series, one colour: a bar's colour names its SERIES, so a From db9e8206ba5a4ee5ca6e4ad5082c26d6f11b12dc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 04:14:41 +0000 Subject: [PATCH 2/4] test(plugin-charts): read the y-axis ticks by their tick-labels group Claude-Session: https://claude.ai/code/session_01JQwxbY75sPQWcfUg8cZQhg Co-authored-by: Claude --- .../src/AdvancedChartImpl.dataLabels-27372.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.dataLabels-27372.test.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.dataLabels-27372.test.tsx index 92775f09eb6..cbb2711be80 100644 --- a/ui/packages/plugin-charts/src/AdvancedChartImpl.dataLabels-27372.test.tsx +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.dataLabels-27372.test.tsx @@ -79,7 +79,7 @@ function labelsOf(container: HTMLElement) { } /** The y-axis tick texts, so a label can be read against its own axis. */ -function yTicksOf(container: HTMLElement, axis = '.recharts-yAxis') { +function yTicksOf(container: HTMLElement, axis = '.recharts-yAxis-tick-labels') { return [...container.querySelectorAll(`${axis} .recharts-cartesian-axis-tick-value`)].map((t) => (t.textContent ?? '').trim(), ); From a3517bca6d6ddae5439b8268eb05444f04c907d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 04:27:48 +0000 Subject: [PATCH 3/4] chore(changeset): plugin-charts patch for data label units and headroom Claude-Session: https://claude.ai/code/session_01JQwxbY75sPQWcfUg8cZQhg Co-authored-by: Claude --- .changeset/27372-chart-data-label-units-headroom.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 .changeset/27372-chart-data-label-units-headroom.md diff --git a/.changeset/27372-chart-data-label-units-headroom.md b/.changeset/27372-chart-data-label-units-headroom.md new file mode 100644 index 00000000000..bd56ea35d67 --- /dev/null +++ b/.changeset/27372-chart-data-label-units-headroom.md @@ -0,0 +1,11 @@ +--- +'@object-ui/plugin-charts': patch +--- + +fix(plugin-charts): a chart's data labels print in their value axis's units, and the tallest mark keeps its label on the chart surface + +Clause-②: no +Metadata-protocol: none + +- **Units.** With `showDataLabels` on, each value label now prints with the tick formatter of the value axis its series is drawn against. An axis with no authored `format` labels its marks in the same compact units as its ticks (`174K` beside a `400K` tick, where the label used to print `174000`); an authored `yAxis[].format` still wins, per axis on a dual-axis or combo chart. Dashboard charts, whose `chartConfig` carries no axis format, are the common case. +- **Headroom.** A vertical bar, line, area or combo chart with data labels on takes a top margin that fits one label above the plot, so a mark on the top edge of the domain (the tallest bar under a nice top tick) no longer draws its label outside the chart. Charts without data labels, and horizontal bars, keep recharts' default margin. From 27cff9de22d5a90be6d3c1ca2f03c1d306a2d8ff Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 04:43:03 +0000 Subject: [PATCH 4/4] refactor(plugin-charts): type the value-label formatter helper by its axis formatter Claude-Session: https://claude.ai/code/session_01JQwxbY75sPQWcfUg8cZQhg Co-authored-by: Claude --- ui/packages/plugin-charts/src/AdvancedChartImpl.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx index dc1e8457ef0..e62118d16d4 100644 --- a/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx +++ b/ui/packages/plugin-charts/src/AdvancedChartImpl.tsx @@ -2226,7 +2226,7 @@ function AdvancedChartImplInner({ * The tick formatter of the value axis a series' marks are measured * against — the one its value labels print with (objectstack#27372). */ - const valueTickFormatterOf = (s: { yAxis?: string }): ((v: any) => string) => + const valueTickFormatterOf = (s: { yAxis?: string }): typeof yTickFormatter => hasDualAxis ? (s.yAxis === 'right' ? rightYTickFormatter : leftYTickFormatter) : yTickFormatter; /**