diff --git a/projects/js-packages/charts/changelog/uni-753-axis-edge-labels b/projects/js-packages/charts/changelog/uni-753-axis-edge-labels new file mode 100644 index 000000000000..ea178eaad327 --- /dev/null +++ b/projects/js-packages/charts/changelog/uni-753-axis-edge-labels @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Reserve room for the first and last labels on a time axis, size the y-axis gutter from a pinned domain, and reserve nothing for a hidden y axis. diff --git a/projects/js-packages/charts/src/charts/pie-chart/test/pie-chart.test.tsx b/projects/js-packages/charts/src/charts/pie-chart/test/pie-chart.test.tsx index 5c59e82a773a..564d7de64851 100644 --- a/projects/js-packages/charts/src/charts/pie-chart/test/pie-chart.test.tsx +++ b/projects/js-packages/charts/src/charts/pie-chart/test/pie-chart.test.tsx @@ -123,18 +123,9 @@ describe( 'PieChart', () => { test( 'hides labels when showLabels is false', () => { renderWithTheme( { showLabels: false } ); - // When showLabels is false, the chart should not display the data labels - // We filter out measurement elements by checking that text is not inside measurement element - const labelElements = screen.queryAllByText( ( content, element ) => { - // Check if this text element is not the measurement element - return ( - ( content === 'A' || content === 'B' ) && - element?.id !== '__react_svg_text_measurement_id' - ); - } ); - - // Labels should not be present in the rendered output (excluding measurement text) - expect( labelElements ).toHaveLength( 0 ); + // A plain query, so this also fails if the measurement node stops being + // ignored — see tests/setup-text-measurement.js. + expect( screen.queryAllByText( /^[AB]$/ ) ).toHaveLength( 0 ); } ); test( 'shows labels when showLabels is explicitly true', () => { diff --git a/projects/js-packages/charts/src/hooks/test/use-chart-margin.test.tsx b/projects/js-packages/charts/src/hooks/test/use-chart-margin.test.tsx index 95f05c73eb2d..aba406b2be5f 100644 --- a/projects/js-packages/charts/src/hooks/test/use-chart-margin.test.tsx +++ b/projects/js-packages/charts/src/hooks/test/use-chart-margin.test.tsx @@ -9,6 +9,13 @@ jest.mock( '../../utils/get-longest-tick-width', () => ( { getLongestTickWidth: ( ...args: unknown[] ) => mockGetLongestTickWidth( ...args ), } ) ); +// jsdom has no getComputedTextLength, so the real measurement always returns null. +const mockGetEdgeTickWidths = jest.fn(); +jest.mock( '../../utils/get-edge-tick-widths', () => ( { + ...jest.requireActual( '../../utils/get-edge-tick-widths' ), + getEdgeTickWidths: ( ...args: unknown[] ) => mockGetEdgeTickWidths( ...args ), +} ) ); + describe( 'useChartMargin', () => { const baseTheme = { axisStyles: { @@ -43,6 +50,8 @@ describe( 'useChartMargin', () => { beforeEach( () => { mockGetLongestTickWidth.mockReset(); mockGetLongestTickWidth.mockReturnValue( 40 ); + mockGetEdgeTickWidths.mockReset(); + mockGetEdgeTickWidths.mockReturnValue( { first: 0, last: 0 } ); } ); it( 'calculates left margin for left y axis', () => { @@ -62,7 +71,7 @@ describe( 'useChartMargin', () => { expect( mockGetLongestTickWidth ).toHaveBeenCalledWith( expect.any( Array ), options.axis.y.tickFormat, - theme.axisStyles.y.left.axisLabel + { fontSize: '12px' } ); // 40 label width + 8 tick length + ceil(11 * 0.25) label dx offset expect( result.current.left ).toBe( 51 ); @@ -85,7 +94,7 @@ describe( 'useChartMargin', () => { expect( mockGetLongestTickWidth ).toHaveBeenCalledWith( expect.any( Array ), options.axis.y.tickFormat, - theme.axisStyles.y.right.axisLabel + { fontSize: '12px' } ); // 40 label width + 8 tick length + ceil(11 * 0.25) label dx offset expect( result.current.right ).toBe( 51 ); @@ -108,7 +117,7 @@ describe( 'useChartMargin', () => { expect( mockGetLongestTickWidth ).toHaveBeenCalledWith( [ 0, 1000 ], options.axis.y.tickFormat, - theme.axisStyles.y.left.axisLabel + { fontSize: '12px' } ); } ); @@ -185,6 +194,167 @@ describe( 'useChartMargin', () => { expect( result.current.bottom ).toBe( 25 ); } ); + describe( 'x-axis edge tick labels', () => { + const tickFormat = ( value: number ) => new Date( value ).toDateString(); + const tickValues = [ 1, 2, 3 ]; + const datedXOptions = ( xOverrides = {} ) => ( { + ...optionsBase, + axis: { ...optionsBase.axis, x: { tickValues, tickFormat, ...xOverrides } }, + } ); + + it( 'reserves half of the last label on the right', () => { + mockGetEdgeTickWidths.mockReturnValue( { first: 0, last: 60 } ); + + const { result } = renderHook( () => + useChartMargin( 300, datedXOptions(), data, baseTheme ) + ); + + expect( result.current.right ).toBe( 30 ); + } ); + + it( 'keeps the default right margin when the last label fits inside it', () => { + mockGetEdgeTickWidths.mockReturnValue( { first: 0, last: 30 } ); + + const { result } = renderHook( () => + useChartMargin( 300, datedXOptions(), data, baseTheme ) + ); + + expect( result.current.right ).toBe( 20 ); + } ); + + it( 'widens the left margin past the y-axis reservation when the first label needs it', () => { + mockGetEdgeTickWidths.mockReturnValue( { first: 120, last: 0 } ); + + const { result } = renderHook( () => + useChartMargin( 300, datedXOptions(), data, baseTheme ) + ); + + // 60 for the label's overhanging half, over the 51 the y-axis ticks need. + expect( result.current.left ).toBe( 60 ); + } ); + + it( 'measures the axis tick values with the x tick label style', () => { + const theme = { + ...baseTheme, + axisStyles: { + ...baseTheme.axisStyles, + x: { + bottom: { tickLabel: { fontSize: 11 }, tickLength: 8 } as unknown as never, + top: {} as unknown as never, + }, + }, + } as XYChartTheme; + + renderHook( () => useChartMargin( 300, datedXOptions(), data, theme ) ); + + expect( mockGetEdgeTickWidths ).toHaveBeenCalledWith( tickValues, tickFormat, { + fontSize: '11px', + } ); + } ); + + it( 'falls back to the raw tick label style when its font size is a relative unit', () => { + const theme = { + ...baseTheme, + axisStyles: { + ...baseTheme.axisStyles, + x: { + bottom: { tickLabel: { fontSize: '0.875rem' }, tickLength: 8 } as unknown as never, + top: {} as unknown as never, + }, + }, + } as XYChartTheme; + + renderHook( () => useChartMargin( 300, datedXOptions(), data, theme ) ); + + expect( mockGetEdgeTickWidths ).toHaveBeenCalledWith( tickValues, tickFormat, { + fontSize: '0.875rem', + } ); + } ); + + it( 'reserves the edge labels on a top x axis too', () => { + mockGetEdgeTickWidths.mockReturnValue( { first: 120, last: 60 } ); + + const { result } = renderHook( () => + useChartMargin( 300, datedXOptions( { orientation: 'top' } ), data, baseTheme ) + ); + + expect( result.current.right ).toBe( 30 ); + expect( result.current.left ).toBe( 60 ); + } ); + + it( 'reserves nothing for a hidden x axis', () => { + mockGetEdgeTickWidths.mockReturnValue( { first: 120, last: 60 } ); + + const { result } = renderHook( () => + useChartMargin( 300, datedXOptions( { display: false } ), data, baseTheme ) + ); + + expect( mockGetEdgeTickWidths ).not.toHaveBeenCalled(); + expect( result.current.right ).toBe( 20 ); + expect( result.current.left ).toBe( 51 ); + } ); + } ); + + describe( 'real measurement', () => { + // Everything else here mocks the measurer out; this block runs it for real, + // so that a width that never reaches the margin would fail something. + const actual = jest.requireActual( '../../utils/get-edge-tick-widths' ); + type Measurable = { getComputedTextLength?: () => number }; + + afterEach( () => { + delete ( window.SVGElement.prototype as Measurable ).getComputedTextLength; + } ); + + it( 'turns a measured edge label into a reserved margin', () => { + // jsdom ships no getComputedTextLength, so @visx/text cannot measure at all. + ( window.SVGElement.prototype as Measurable ).getComputedTextLength = function ( + this: SVGElement + ) { + return ( this.textContent ?? '' ).length * 8; + }; + mockGetEdgeTickWidths.mockImplementation( actual.getEdgeTickWidths ); + + const options = { + ...optionsBase, + axis: { + ...optionsBase.axis, + x: { + tickValues: [ 1, 2 ], + tickFormat: ( _value: number, index: number ) => + index === 0 ? 'AA' : 'MEASURED-LAST', + }, + }, + }; + + const { result } = renderHook( () => useChartMargin( 300, options, data, baseTheme ) ); + + // 'MEASURED-LAST' is 13 characters, so 104px wide, and half of it is reserved. + expect( result.current.right ).toBe( 52 ); + } ); + } ); + + describe( 'y axis gutter', () => { + it( 'measures a caller-pinned domain rather than the data range', () => { + const options = { ...optionsBase, yScale: { domain: [ 0, 1 ] as [ number, number ] } }; + + renderHook( () => useChartMargin( 300, options, data, baseTheme ) ); + + const ticks = mockGetLongestTickWidth.mock.calls[ 0 ][ 0 ] as number[]; + expect( Math.max( ...ticks ) ).toBeLessThanOrEqual( 1 ); + } ); + + it( 'reserves no gutter for a hidden y axis', () => { + const options = { + ...optionsBase, + axis: { ...optionsBase.axis, y: { ...optionsBase.axis.y, display: false } }, + }; + + const { result } = renderHook( () => useChartMargin( 300, options, data, baseTheme ) ); + + expect( result.current.left ).toBe( 20 ); + } ); + } ); + describe( 'horizontal y ticks', () => { const horizontalOptions = ( tickFormat: ( value: string | number ) => string ) => ( { ...optionsBase, diff --git a/projects/js-packages/charts/src/hooks/use-chart-margin.tsx b/projects/js-packages/charts/src/hooks/use-chart-margin.tsx index 7b022ddf0ec0..3715d628f444 100644 --- a/projects/js-packages/charts/src/hooks/use-chart-margin.tsx +++ b/projects/js-packages/charts/src/hooks/use-chart-margin.tsx @@ -1,6 +1,6 @@ import { createScale, getTicks } from '@visx/scale'; import { useMemo } from 'react'; -import { getLongestTickWidth, resolveFontSize } from '../utils'; +import { getEdgeTickWidths, getLongestTickWidth, resolveFontSize } from '../utils'; import type { BaseChartProps, DataPointDate, SeriesData } from '../types'; import type { XYChartTheme } from '@visx/xychart'; @@ -50,6 +50,33 @@ const DEFAULT_TICK_LENGTH = 8; */ const DEFAULT_Y_TICK_WIDTH = 40; +type LabelStyle = { fontSize?: number | string; letterSpacing?: number | string }; + +/** + * Copy a label style with its lengths spelled out in px. + * + * `getStringWidth` applies the style through CSSOM. Blink resolves a bare number + * on an SVG ``, but that is its own leniency rather than the CSS rule, and + * `buildChartTheme` hands us bare numbers. + * + * @param style - Raw label style from the theme. + * @return The same style with px-qualified lengths. + */ +const toMeasurableStyle = < T extends LabelStyle >( style?: T ) => { + if ( ! style ) { + return style; + } + + const fontSize = resolveFontSize( style.fontSize ); + const { letterSpacing } = style; + + return { + ...style, + ...( fontSize === undefined ? {} : { fontSize: `${ fontSize }px` } ), + ...( typeof letterSpacing === 'number' ? { letterSpacing: `${ letterSpacing }px` } : {} ), + }; +}; + const getXAxisLabelMetrics = ( theme: XYChartTheme, orientation: 'top' | 'bottom' ) => { const xAxisStyles = orientation === 'top' ? theme.axisStyles?.x?.top : theme.axisStyles?.x?.bottom; @@ -61,7 +88,7 @@ const getXAxisLabelMetrics = ( theme: XYChartTheme, orientation: 'top' | 'bottom const tickLength = xAxisStyles?.tickLength ?? DEFAULT_TICK_LENGTH; - return { fontSize, tickLength }; + return { fontSize, tickLength, tickLabelStyle: toMeasurableStyle( xAxisStyles?.tickLabel ) }; }; export const useChartMargin = ( @@ -88,7 +115,9 @@ export const useChartMargin = ( const maxY = Math.max( ...allDataPoints.map( d => d.value ) ); const yScale = createScale( { ...options.yScale, - domain: [ minY, maxY ], + // A pinned domain is what the axis actually renders, so measure those + // ticks; the data's range would size the gutter for narrower labels. + domain: options.yScale?.domain ?? [ minY, maxY ], range: [ height, 0 ], } ); @@ -111,7 +140,7 @@ export const useChartMargin = ( const yTickWidth = getLongestTickWidth( yTicks, options.axis?.y?.tickFormat, - yAxisStyles.axisLabel + toMeasurableStyle( yAxisStyles.axisLabel ) ); // visx's default axis theme pushes y-axis tick labels a further 0.25em // away from the axis (dx of -0.25em on the left, 0.25em on the right), so @@ -125,17 +154,20 @@ export const useChartMargin = ( ( yAxisStyles?.tickLength ?? 0 ) + Math.ceil( yTickLabelFontSize * 0.25 ); - if ( yAxisOrientation === 'right' ) { - defaultMargin.right = yMarginValue; - } else { - defaultMargin.left = yMarginValue; + // A hidden y axis reserves nothing; its gutter belongs to the plot area. + if ( options.axis?.y?.display !== false ) { + if ( yAxisOrientation === 'right' ) { + defaultMargin.right = yMarginValue; + } else { + defaultMargin.left = yMarginValue; + } } // Dynamically compute X-axis margin (bottom by default, or top if orientation is 'top'). // This mirrors Y-axis behavior where margin is based on label size and tick length, // but keeps the padding minimal so consumers can control container spacing themselves. const xOrientation = options.axis?.x?.orientation === 'top' ? 'top' : 'bottom'; - const { fontSize, tickLength } = getXAxisLabelMetrics( theme, xOrientation ); + const { fontSize, tickLength, tickLabelStyle } = getXAxisLabelMetrics( theme, xOrientation ); const computedXMargin = fontSize + tickLength; if ( xOrientation === 'top' ) { @@ -145,6 +177,19 @@ export const useChartMargin = ( defaultMargin.bottom = Math.max( defaultMargin.bottom, computedXMargin ); } + // An X-axis label is centered on its tick, so the ones at either end of the + // scale hang half their width outside the plot area and clip at the SVG edge. + if ( options.axis?.x?.display !== false ) { + const { first, last } = getEdgeTickWidths( + options.axis?.x?.tickValues ?? [], + options.axis?.x?.tickFormat, + tickLabelStyle + ); + + defaultMargin.left = Math.max( defaultMargin.left, Math.ceil( first / 2 ) ); + defaultMargin.right = Math.max( defaultMargin.right, Math.ceil( last / 2 ) ); + } + return defaultMargin; }, [ options, theme, yTicks ] ); }; diff --git a/projects/js-packages/charts/src/utils/get-edge-tick-widths.ts b/projects/js-packages/charts/src/utils/get-edge-tick-widths.ts new file mode 100644 index 000000000000..7107561b2447 --- /dev/null +++ b/projects/js-packages/charts/src/utils/get-edge-tick-widths.ts @@ -0,0 +1,33 @@ +import { getStringWidth } from '@visx/text'; +import type { TickFormatter } from '@visx/axis'; +import type { AnyD3Scale, ScaleInput } from '@visx/scale'; + +/** + * Rendered widths of the first and last tick labels on an axis. + * + * An unmeasurable label reserves nothing, which is what a width of 0 already + * means to every caller, so it is reported as 0 rather than as its own case. + * + * @param ticks - Tick values, in axis order. + * @param formatTick - Function to format a tick. + * @param {object} labelStyle - Style object for the label. + * @return {object} - Widths in pixels. + */ +export const getEdgeTickWidths = < T extends AnyD3Scale >( + ticks: ScaleInput< T >[], + formatTick?: TickFormatter< ScaleInput< T > >, + labelStyle?: object +): { first: number; last: number } => { + if ( ! ticks.length ) { + return { first: 0, last: 0 }; + } + + const lastIndex = ticks.length - 1; + const label = ( tick: ScaleInput< T >, index: number ) => + String( formatTick ? formatTick( tick, index, [] ) ?? '' : tick ); + + return { + first: getStringWidth( label( ticks[ 0 ], 0 ), labelStyle ) ?? 0, + last: getStringWidth( label( ticks[ lastIndex ], lastIndex ), labelStyle ) ?? 0, + }; +}; diff --git a/projects/js-packages/charts/src/utils/index.ts b/projects/js-packages/charts/src/utils/index.ts index 3ffe35a2c60c..6bf1aed84a5a 100644 --- a/projects/js-packages/charts/src/utils/index.ts +++ b/projects/js-packages/charts/src/utils/index.ts @@ -13,6 +13,7 @@ export type { MetricValueType } from './format-metric-value'; export { formatPercentage } from './format-percentage'; // Chart measurement utilities +export { getEdgeTickWidths } from './get-edge-tick-widths'; export { getLongestTickWidth } from './get-longest-tick-width'; // Style and theming utilities diff --git a/projects/js-packages/charts/src/utils/test/get-edge-tick-widths.test.ts b/projects/js-packages/charts/src/utils/test/get-edge-tick-widths.test.ts new file mode 100644 index 000000000000..f1900e08c43b --- /dev/null +++ b/projects/js-packages/charts/src/utils/test/get-edge-tick-widths.test.ts @@ -0,0 +1,55 @@ +import { getEdgeTickWidths } from '../get-edge-tick-widths'; + +const mockGetStringWidth = jest.fn(); +jest.mock( '@visx/text', () => ( { + ...jest.requireActual( '@visx/text' ), + getStringWidth: ( ...args: unknown[] ) => mockGetStringWidth( ...args ), +} ) ); + +describe( 'getEdgeTickWidths', () => { + beforeEach( () => { + mockGetStringWidth.mockReset(); + mockGetStringWidth.mockImplementation( ( label: string ) => label.length ); + } ); + + it( 'measures the formatted first and last ticks', () => { + const style = { fontSize: 11 }; + + const widths = getEdgeTickWidths( [ 1, 2, 3 ], value => `tick-${ value }`, style ); + + expect( widths ).toEqual( { first: 6, last: 6 } ); + expect( mockGetStringWidth ).toHaveBeenCalledWith( 'tick-1', style ); + expect( mockGetStringWidth ).toHaveBeenCalledWith( 'tick-3', style ); + } ); + + it( 'passes each tick its own index', () => { + getEdgeTickWidths( [ 'a', 'b', 'c' ], ( value, index ) => `${ index }:${ value }` ); + + expect( mockGetStringWidth ).toHaveBeenCalledWith( '0:a', undefined ); + expect( mockGetStringWidth ).toHaveBeenCalledWith( '2:c', undefined ); + } ); + + it( 'measures a single tick as both edges', () => { + expect( getEdgeTickWidths( [ 42 ], value => `${ value }` ) ).toEqual( { first: 2, last: 2 } ); + } ); + + it( 'measures the raw value when there is no formatter', () => { + getEdgeTickWidths( [ 'abcd' ] ); + + expect( mockGetStringWidth ).toHaveBeenCalledWith( 'abcd', undefined ); + } ); + + it( 'returns nothing to reserve for an axis with no ticks', () => { + expect( getEdgeTickWidths( [] ) ).toEqual( { first: 0, last: 0 } ); + expect( mockGetStringWidth ).not.toHaveBeenCalled(); + } ); + + it( 'reserves nothing for a label the measurer cannot size', () => { + mockGetStringWidth.mockReturnValue( null ); + + expect( getEdgeTickWidths( [ 1, 2, 3 ], value => `tick-${ value }` ) ).toEqual( { + first: 0, + last: 0, + } ); + } ); +} ); diff --git a/projects/js-packages/charts/tests/jest.config.cjs b/projects/js-packages/charts/tests/jest.config.cjs index 19555a4c2405..bea985acf1ee 100644 --- a/projects/js-packages/charts/tests/jest.config.cjs +++ b/projects/js-packages/charts/tests/jest.config.cjs @@ -21,5 +21,6 @@ module.exports = { setupFilesAfterEnv: [ ...( baseConfig.setupFilesAfterEnv || [] ), path.join( __dirname, 'setup-element-size-mock.js' ), + path.join( __dirname, 'setup-text-measurement.js' ), ], }; diff --git a/projects/js-packages/charts/tests/setup-text-measurement.js b/projects/js-packages/charts/tests/setup-text-measurement.js new file mode 100644 index 000000000000..60846e1e96f3 --- /dev/null +++ b/projects/js-packages/charts/tests/setup-text-measurement.js @@ -0,0 +1,12 @@ +/** + * Hide visx's text-measurement node from Testing Library's text queries. + * + * `@visx/text`'s getStringWidth measures by parking the string in a shared, + * offscreen node on document.body and leaving it there. Whatever the + * axis measured last therefore answers `getByText` a second time — an axis + * label measured for the chart margin collides with the tick that renders it. + */ + +const { configure } = require( '@testing-library/react' ); + +configure( { defaultIgnore: 'script, style, #__react_svg_text_measurement_id' } ); diff --git a/projects/packages/premium-analytics/changelog/uni-753-axis-edge-labels b/projects/packages/premium-analytics/changelog/uni-753-axis-edge-labels new file mode 100644 index 000000000000..f3793536e488 --- /dev/null +++ b/projects/packages/premium-analytics/changelog/uni-753-axis-edge-labels @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Stop the first and last dates on a chart's horizontal axis from being cut off. diff --git a/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/README.md b/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/README.md index cfe97528f85c..6876f1059449 100644 --- a/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/README.md +++ b/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/README.md @@ -56,6 +56,5 @@ unreadable. ## Y-axis domain Percentage metrics are pinned to 0%–100% and an all-zero period gets a readable axis instead of a -flat baseline, both via the shared `getFixedYAxis` helper, which also supplies the left margin such -a pinned domain needs. Zero-value bars are drawn as hairline stubs (`showZeroValues`) so a quiet day -reads as zero rather than missing data. +flat baseline, both via the shared `getFixedYAxis` helper. Zero-value bars are drawn as hairline +stubs (`showZeroValues`) so a quiet day reads as zero rather than missing data. diff --git a/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/__tests__/comparative-bar-chart.test.tsx b/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/__tests__/comparative-bar-chart.test.tsx index bd7939d69d67..9e13946c2ed4 100644 --- a/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/__tests__/comparative-bar-chart.test.tsx +++ b/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/__tests__/comparative-bar-chart.test.tsx @@ -478,19 +478,21 @@ describe( 'ComparativeBarChart', () => { expect( recordedOptions().yScale?.domain ).toEqual( [ 0, 1 ] ); } ); - it( 'reserves a left margin for a pinned domain', () => { + it( 'leaves the pinned domain to size its own gutter', () => { render( ); - // `useChartMargin` sizes the gutter from the data's own min/max, so the - // pinned domain's widest tick would otherwise be clipped. - expect( recordedProps().margin.left ).toBeGreaterThan( 0 ); + // `useChartMargin` measures the pinned domain's own ticks, so there is + // nothing left for this component to override. + expect( recordedOptions().yScale.domain ).toBeDefined(); + expect( recordedProps().margin ).toBeUndefined(); } ); it( 'lets the chart scale to the data otherwise', () => { render( ); expect( recordedOptions() ).not.toHaveProperty( 'yScale' ); - expect( recordedProps().margin ).toEqual( { right: 0 } ); + // No override, so the chart keeps the gutters `useChartMargin` measured. + expect( recordedProps().margin ).toBeUndefined(); } ); } ); @@ -503,8 +505,9 @@ describe( 'ComparativeBarChart', () => { expect( recordedOptions().axis.y.display ).toBe( false ); expect( recordedProps().gridVisibility ).toBe( 'none' ); - // The hidden axis frees its gutter for the bars. - expect( recordedProps().margin ).toEqual( { right: 0, left: 0 } ); + // The hidden axis frees its gutter inside `useChartMargin`, so the bars + // gain the room without this component clipping the date labels away. + expect( recordedProps().margin ).toBeUndefined(); expect( screen.queryByTestId( 'bar-chart-legend' ) ).not.toBeInTheDocument(); } ); diff --git a/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/comparative-bar-chart.tsx b/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/comparative-bar-chart.tsx index c1a82f128371..17617a4313ce 100644 --- a/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/comparative-bar-chart.tsx +++ b/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-bar/comparative-bar-chart.tsx @@ -36,9 +36,6 @@ import type { ComparativeDatePointDate } from '../chart-comparative-line/types'; import type { TooltipStyle } from '../chart-tooltip'; import type { ComponentProps } from 'react'; -/** The y-axis is on the left, so the right margin is always 0. */ -const DEFAULT_MARGIN = { right: 0 }; - /** * Chart-area height (px) below which `compactWhenShort` degrades the chart to * a sparkline (no y-axis, grid, or legend). Matches the comparative line chart @@ -289,12 +286,12 @@ export function ComparativeBarChart( { ); /** - * A pinned domain for percentage metrics and all-zero periods, with the left - * margin its widest tick needs. Null lets the chart scale to the data. + * A pinned domain for percentage metrics and all-zero periods. Null lets the + * chart scale to the data. */ const fixedYAxis = useMemo( - () => getFixedYAxis( dataFormat.type, isEmptyData, yTickFormat ), - [ dataFormat.type, isEmptyData, yTickFormat ] + () => getFixedYAxis( dataFormat.type, isEmptyData ), + [ dataFormat.type, isEmptyData ] ); const chartOptions = useMemo( () => { @@ -319,15 +316,6 @@ export function ComparativeBarChart( { return { ...baseOptions, yScale: { domain: fixedYAxis.domain } }; }, [ xTickFormat, tickResolution, yTickFormat, isCompact, fixedYAxis ] ); - const margin = useMemo( () => { - // With the y-axis hidden, reclaim its reserved left margin for the bars. - if ( isCompact ) { - return { ...DEFAULT_MARGIN, left: 0 }; - } - - return fixedYAxis ? { ...DEFAULT_MARGIN, left: fixedYAxis.marginLeft } : DEFAULT_MARGIN; - }, [ isCompact, fixedYAxis ] ); - return ( { }; } ); +// jsdom's ResizeObserver is a no-op stub, so the real hook's callback never fires +// and the chart measures as infinitely tall, leaving `compactWhenShort` unreachable. +let mockChartHeight = Infinity; + jest.mock( '@wordpress/compose', () => ( { ...jest.requireActual( '@wordpress/compose' ), - useResizeObserver: () => () => undefined, + useResizeObserver: + ( onResize: ( entries: { contentRect: { height: number } }[] ) => void ) => + ( element: HTMLElement | null ) => { + if ( element ) { + onResize( [ { contentRect: { height: mockChartHeight } } ] ); + } + }, } ) ); jest.mock( '../../../hooks', () => ( { @@ -132,6 +142,8 @@ type RecordedLineProps = { chartId?: string; defaultHiddenSeries?: readonly string[]; legend: { collapseGroups: boolean; interactive: boolean }; + margin?: Record< string, number >; + options?: { yScale?: { domain?: [ number, number ] }; axis: { y: { display?: boolean } } }; renderTooltip: ( params: unknown ) => { props: { getLabel: GetTooltipLabel } }; }; @@ -176,6 +188,42 @@ describe( 'ComparativeLineChart', () => { beforeEach( () => { mockLineSpy.mockClear(); mockLegendSpy.mockClear(); + mockChartHeight = Infinity; + } ); + + describe( 'margin', () => { + it( 'never overrides the gutters the chart measured', () => { + render( ); + + expect( recordedProps().margin ).toBeUndefined(); + } ); + + it( 'leaves the pinned domain to size its own gutter', () => { + render( + + ); + + // `useChartMargin` measures the pinned domain's own ticks, so there is + // nothing left for this component to override. + expect( recordedProps().options.yScale.domain ).toBeDefined(); + expect( recordedProps().margin ).toBeUndefined(); + } ); + + it( 'keeps the date labels on a sparkline', () => { + mockChartHeight = 80; + + render( + + ); + + // The hidden y axis frees its gutter inside `useChartMargin`; zeroing the + // margin here would clip the first and last dates, which still render. + expect( recordedProps().options.axis.y.display ).toBe( false ); + expect( recordedProps().margin ).toBeUndefined(); + } ); } ); it( 'passes visibility settings through to the chart and legend', () => { diff --git a/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-line/comparative-line-chart.tsx b/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-line/comparative-line-chart.tsx index 124a60c3e033..012d5cc312e5 100644 --- a/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-line/comparative-line-chart.tsx +++ b/projects/packages/premium-analytics/packages/widgets-toolkit/src/components/chart-comparative-line/comparative-line-chart.tsx @@ -56,9 +56,6 @@ function resolveSeriesStyles( } ); } -/** The y-axis is on the left, so the right margin is always 0. */ -const DEFAULT_MARGIN = { right: 0 }; - /** * Chart-area height (px) below which `compactWhenShort` degrades the chart to * a sparkline (no y-axis, grid, or legend). @@ -245,11 +242,11 @@ export function ComparativeLineChart( { const isEmptyData = useMemo( () => isEmptyChartData( styledSeries ), [ styledSeries ] ); - // A pinned domain for percentage metrics and all-zero periods, with the left - // margin its widest tick needs. Null lets the chart scale to the data. + // A pinned domain for percentage metrics and all-zero periods. Null lets the + // chart scale to the data. const fixedYAxis = useMemo( - () => getFixedYAxis( dataFormat.type, isEmptyData, yTickFormat ), - [ dataFormat.type, isEmptyData, yTickFormat ] + () => getFixedYAxis( dataFormat.type, isEmptyData ), + [ dataFormat.type, isEmptyData ] ); const xTickFormat = useCallback( @@ -281,8 +278,6 @@ export function ComparativeLineChart( { return { ...baseOptions, yScale: { domain: fixedYAxis.domain } }; }, [ xTickFormat, xTickFormatType, tickResolution, yTickFormat, fixedYAxis, isCompact ] ); - const margin = fixedYAxis ? { ...DEFAULT_MARGIN, left: fixedYAxis.marginLeft } : DEFAULT_MARGIN; - return ( ( +
+ +
+ ), + ], + args: { + series: singleSeries, + styles: SERIES_STYLES, + dataFormat: { type: 'currency' }, + compactWhenShort: true, + }, +}; diff --git a/projects/packages/premium-analytics/packages/widgets-toolkit/src/helpers/fixed-y-axis.ts b/projects/packages/premium-analytics/packages/widgets-toolkit/src/helpers/fixed-y-axis.ts index 94745652f66b..ccd1bf9b647e 100644 --- a/projects/packages/premium-analytics/packages/widgets-toolkit/src/helpers/fixed-y-axis.ts +++ b/projects/packages/premium-analytics/packages/widgets-toolkit/src/helpers/fixed-y-axis.ts @@ -4,32 +4,23 @@ import { getEmptyChartDomain } from './chart-empty-state'; /** - * A y-axis domain pinned by the chart rather than derived from the data, plus - * the left margin that domain needs. + * A y-axis domain pinned by the chart rather than derived from the data. */ export type FixedYAxis = { /** Y-axis domain tuple [min, max]. */ domain: [ number, number ]; - /** Left margin, in px, wide enough for the domain's longest tick label. */ - marginLeft: number; }; /** * Resolve the y-axis domain a comparative chart should pin, if any: a percentage - * metric always reads 0–100%, an all-zero period gets a real axis instead of a - * flat baseline, and the domain carries its own margin since `useChartMargin`'s - * data-derived ticks would otherwise clip a pinned domain's widest tick. + * metric always reads 0–100%, and an all-zero period gets a real axis instead of + * a flat baseline. * * @param metricType - The data format type (currency, number, percentage). * @param isEmptyData - Whether every value in the chart is 0 or null. - * @param formatTick - The chart's y-axis tick formatter, used to size the margin. - * @return The domain and its margin, or null to let the chart scale to the data. + * @return The domain, or null to let the chart scale to the data. */ -export function getFixedYAxis( - metricType: string, - isEmptyData: boolean, - formatTick: ( value: number ) => string -): FixedYAxis | null { +export function getFixedYAxis( metricType: string, isEmptyData: boolean ): FixedYAxis | null { let domain: [ number, number ] | null = null; if ( metricType === 'percentage' ) { @@ -42,7 +33,5 @@ export function getFixedYAxis( return null; } - // Rough but stable: the chart library gives us no way to measure the rendered - // tick, so estimate from the formatted string's length. - return { domain, marginLeft: formatTick( domain[ 1 ] ).length * 10 }; + return { domain }; } diff --git a/projects/plugins/jetpack/changelog/uni-753-axis-edge-labels b/projects/plugins/jetpack/changelog/uni-753-axis-edge-labels new file mode 100644 index 000000000000..9b1e58a4901a --- /dev/null +++ b/projects/plugins/jetpack/changelog/uni-753-axis-edge-labels @@ -0,0 +1,4 @@ +Significance: patch +Type: bugfix + +Charts: Stop the first and last dates on a chart's horizontal axis from being cut off. diff --git a/projects/plugins/premium-analytics/changelog/uni-753-axis-edge-labels b/projects/plugins/premium-analytics/changelog/uni-753-axis-edge-labels new file mode 100644 index 000000000000..f3793536e488 --- /dev/null +++ b/projects/plugins/premium-analytics/changelog/uni-753-axis-edge-labels @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Stop the first and last dates on a chart's horizontal axis from being cut off. diff --git a/projects/plugins/social/changelog/uni-753-axis-edge-labels b/projects/plugins/social/changelog/uni-753-axis-edge-labels new file mode 100644 index 000000000000..494d9b824eca --- /dev/null +++ b/projects/plugins/social/changelog/uni-753-axis-edge-labels @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Charts: Stop the first and last dates on a chart's horizontal axis from being cut off. diff --git a/projects/plugins/videopress/changelog/uni-753-axis-edge-labels b/projects/plugins/videopress/changelog/uni-753-axis-edge-labels new file mode 100644 index 000000000000..494d9b824eca --- /dev/null +++ b/projects/plugins/videopress/changelog/uni-753-axis-edge-labels @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Charts: Stop the first and last dates on a chart's horizontal axis from being cut off.