diff --git a/projects/js-packages/charts/changelog/charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and b/projects/js-packages/charts/changelog/charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and new file mode 100644 index 00000000000..6978eb2f293 --- /dev/null +++ b/projects/js-packages/charts/changelog/charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Charts: fix chart height and size calculations for Pie Chart and variants. diff --git a/projects/js-packages/charts/src/charts/bar-chart/bar-chart.tsx b/projects/js-packages/charts/src/charts/bar-chart/bar-chart.tsx index d77cce9eaed..80ba480c0b1 100644 --- a/projects/js-packages/charts/src/charts/bar-chart/bar-chart.tsx +++ b/projects/js-packages/charts/src/charts/bar-chart/bar-chart.tsx @@ -12,7 +12,7 @@ import { useChartDataTransform, useZeroValueDisplay, useChartMargin, - useElementHeight, + useElementSize, useHasLegendChild, usePrefersReducedMotion, } from '../../hooks'; @@ -125,7 +125,7 @@ const BarChartInternal: FC< BarChartProps > = ( { const legendItems = useChartLegendItems( dataSorted ); const chartOptions = useBarChartOptions( dataWithVisibleZeros, horizontal, options ); const defaultMargin = useChartMargin( height, chartOptions, dataSorted, theme, horizontal ); - const [ svgWrapperRef, svgWrapperHeight ] = useElementHeight< HTMLDivElement >(); + const [ svgWrapperRef, , svgWrapperHeight ] = useElementSize< HTMLDivElement >(); const chartRef = useRef< HTMLDivElement >( null ); // Check if children contain a Legend component (composition pattern) diff --git a/projects/js-packages/charts/src/charts/bar-chart/test/bar-chart.test.tsx b/projects/js-packages/charts/src/charts/bar-chart/test/bar-chart.test.tsx index 2c609270a9b..d3209c194f3 100644 --- a/projects/js-packages/charts/src/charts/bar-chart/test/bar-chart.test.tsx +++ b/projects/js-packages/charts/src/charts/bar-chart/test/bar-chart.test.tsx @@ -3,10 +3,10 @@ import userEvent from '@testing-library/user-event'; import { GlobalChartsProvider } from '../../../providers'; import BarChart from '../bar-chart'; -// Mock useElementHeight to return a non-zero height in jsdom so charts render +// Mock useElementSize to return non-zero dimensions in jsdom so charts render const mockRefCallback = jest.fn(); -jest.mock( '../../../hooks/use-element-height', () => ( { - useElementHeight: () => [ mockRefCallback, 300 ], // Return test height to allow chart rendering +jest.mock( '../../../hooks/use-element-size', () => ( { + useElementSize: () => [ mockRefCallback, 500, 300 ], } ) ); describe( 'BarChart', () => { diff --git a/projects/js-packages/charts/src/charts/line-chart/line-chart.tsx b/projects/js-packages/charts/src/charts/line-chart/line-chart.tsx index 89336865dad..fc4d5fb7d75 100644 --- a/projects/js-packages/charts/src/charts/line-chart/line-chart.tsx +++ b/projects/js-packages/charts/src/charts/line-chart/line-chart.tsx @@ -14,7 +14,7 @@ import { useXYChartTheme, useChartDataTransform, useChartMargin, - useElementHeight, + useElementSize, useHasLegendChild, usePrefersReducedMotion, } from '../../hooks'; @@ -288,7 +288,7 @@ const LineChartInternal = forwardRef< SingleChartRef, LineChartProps >( const providerTheme = useGlobalChartsTheme(); const theme = useXYChartTheme( data ); const chartId = useChartId( providedChartId ); - const [ svgWrapperRef, svgWrapperHeight ] = useElementHeight< HTMLDivElement >(); + const [ svgWrapperRef, , svgWrapperHeight ] = useElementSize< HTMLDivElement >(); const chartRef = useRef< HTMLDivElement >( null ); const [ selectedIndex, setSelectedIndex ] = useState< number | undefined >( undefined ); const [ isNavigating, setIsNavigating ] = useState( false ); diff --git a/projects/js-packages/charts/src/charts/line-chart/test/line-chart.test.tsx b/projects/js-packages/charts/src/charts/line-chart/test/line-chart.test.tsx index f395f7c90c4..2da8f848971 100644 --- a/projects/js-packages/charts/src/charts/line-chart/test/line-chart.test.tsx +++ b/projects/js-packages/charts/src/charts/line-chart/test/line-chart.test.tsx @@ -8,10 +8,10 @@ import { GlobalChartsProvider, defaultTheme } from '../../../providers'; import LineChart, { LineChartUnresponsive } from '../line-chart'; import type { SingleChartRef } from '../../private/single-chart-context'; -// Mock useElementHeight to return a non-zero height in jsdom so charts render +// Mock useElementSize to return non-zero dimensions in jsdom so charts render const mockRefCallback = jest.fn(); -jest.mock( '../../../hooks/use-element-height', () => ( { - useElementHeight: () => [ mockRefCallback, 300 ], // Return test height to allow chart rendering +jest.mock( '../../../hooks/use-element-size', () => ( { + useElementSize: () => [ mockRefCallback, 500, 300 ], } ) ); const customTheme = { diff --git a/projects/js-packages/charts/src/charts/pie-chart/pie-chart.module.scss b/projects/js-packages/charts/src/charts/pie-chart/pie-chart.module.scss index a53f79af2f7..ff3482db7ff 100644 --- a/projects/js-packages/charts/src/charts/pie-chart/pie-chart.module.scss +++ b/projects/js-packages/charts/src/charts/pie-chart/pie-chart.module.scss @@ -3,9 +3,20 @@ flex-direction: column; overflow: hidden; align-items: center; - gap: 20px; - &--legend-top { - flex-direction: column-reverse; + // Fill parent when no explicit width/height provided + &--responsive { + height: 100%; + width: 100%; + } + + &__svg-wrapper { + flex: 1; + min-height: 0; // Required for flex shrinking + min-width: 0; // Required for flex shrinking + width: 100%; + display: flex; + align-items: center; + justify-content: center; } } diff --git a/projects/js-packages/charts/src/charts/pie-chart/pie-chart.tsx b/projects/js-packages/charts/src/charts/pie-chart/pie-chart.tsx index 3e515f98a32..a005298d67b 100644 --- a/projects/js-packages/charts/src/charts/pie-chart/pie-chart.tsx +++ b/projects/js-packages/charts/src/charts/pie-chart/pie-chart.tsx @@ -2,11 +2,12 @@ import { Group } from '@visx/group'; import { Pie } from '@visx/shape'; import { useTooltip, useTooltipInPortal } from '@visx/tooltip'; import { __ } from '@wordpress/i18n'; +import { Stack } from '@wordpress/ui'; import clsx from 'clsx'; import { useCallback, useContext, useMemo } from 'react'; import { Legend, useChartLegendItems } from '../../components/legend'; import { BaseTooltip } from '../../components/tooltip'; -import { useElementHeight, useInteractiveLegendData, usePrefersReducedMotion } from '../../hooks'; +import { useElementSize, useInteractiveLegendData, usePrefersReducedMotion } from '../../hooks'; import { GlobalChartsProvider, useChartId, @@ -25,6 +26,7 @@ import styles from './pie-chart.module.scss'; import type { LegendValueDisplay } from '../../components/legend'; import type { BaseChartProps, DataPointPercentage, Optional } from '../../types'; import type { ChartComponentWithComposition } from '../private/chart-composition'; +import type { GapSize } from '@wordpress/theme'; import type { SVGProps, MouseEvent, ReactNode, FC } from 'react'; /** @@ -119,6 +121,13 @@ export interface PieChartProps extends BaseChartProps< DataPointPercentage[] > { * When provided, replaces the default BaseTooltip with custom content. */ renderTooltip?: ( params: PieChartRenderTooltipParams ) => ReactNode; + + /** + * Gap between chart elements (SVG, legend, children). + * Uses WordPress design system tokens. + * @default 'md' + */ + gap?: GapSize; } // Base props type with optional responsive properties @@ -175,6 +184,8 @@ const PieChartInternal = ( { legendTextOverflow = 'wrap', legendItemClassName, legendShape = 'circle', + width: propWidth, + height: propHeight, size, animation, thickness = 1, @@ -188,10 +199,11 @@ const PieChartInternal = ( { tooltipOffsetX = 0, tooltipOffsetY = -15, renderTooltip = renderDefaultPieTooltip, + gap = 'md', }: PieChartProps ) => { const providerTheme = useGlobalChartsTheme(); const chartId = useChartId( providedChartId ); - const [ legendRef, legendHeight ] = useElementHeight< HTMLDivElement >(); + const [ svgWrapperRef, svgWrapperWidth, svgWrapperHeight ] = useElementSize< HTMLDivElement >(); const { tooltipOpen, tooltipLeft, tooltipTop, tooltipData, hideTooltip, showTooltip } = useTooltip< DataPointPercentage >(); @@ -263,16 +275,24 @@ const PieChartInternal = ( { ); } - const width = size; - const height = size; - const adjustedHeight = showLegend && legendPosition === 'top' ? height - legendHeight : height; + // Calculate the actual pie size: + // - Measure available space from the svg-wrapper + // - If size prop provided: use it as max, but shrink if container is smaller + // - If no size prop: fill available space + const availableWidth = svgWrapperWidth > 0 ? svgWrapperWidth : 300; + const availableHeight = svgWrapperHeight > 0 ? svgWrapperHeight : 300; + const availableSize = Math.min( availableWidth, availableHeight ); + const actualSize = size ? Math.min( size, availableSize ) : availableSize; + + const width = actualSize; + const height = actualSize; // Calculate radius based on width/height - const radius = Math.min( width, adjustedHeight ) / 2; + const radius = Math.min( width, height ) / 2; // Center the chart in the available space const centerX = width / 2; - const centerY = adjustedHeight / 2; + const centerY = height / 2; // Calculate the angle between each (use original data length for consistent spacing) const padAngle = gapScale * ( ( 2 * Math.PI ) / data.length ); @@ -300,166 +320,178 @@ const PieChartInternal = ( { }, }; + const legendElement = showLegend && ( + + ); + return ( -
- - - - - - + - { allSegmentsHidden ? ( - - { __( - 'All segments are hidden. Click legend items to show data.', - 'jetpack-charts' - ) } - - ) : ( - - data={ dataWithIndex } - pieValue={ accessors.value } - outerRadius={ outerRadius } + + - { pie => { - return pie.arcs.map( ( arc, index ) => { - const [ centroidX, centroidY ] = pie.path.centroid( arc ); - const hasSpaceForLabel = arc.endAngle - arc.startAngle >= 0.25; - const handleMouseMove = ( event: MouseEvent< SVGElement > ) => { - if ( ! withTooltips ) { - return; - } - - // Don't show tooltip until container bounds are measured - if ( containerBounds.width === 0 || containerBounds.height === 0 ) { - return; + /> + + + + { allSegmentsHidden ? ( + + { __( + 'All segments are hidden. Click legend items to show data.', + 'jetpack-charts' + ) } + + ) : ( + + data={ dataWithIndex } + pieValue={ accessors.value } + outerRadius={ outerRadius } + innerRadius={ innerRadius } + padAngle={ padAngle } + cornerRadius={ cornerRadius } + > + { pie => { + return pie.arcs.map( ( arc, index ) => { + const [ centroidX, centroidY ] = pie.path.centroid( arc ); + const hasSpaceForLabel = arc.endAngle - arc.startAngle >= 0.25; + const handleMouseMove = ( event: MouseEvent< SVGElement > ) => { + if ( ! withTooltips ) { + return; + } + + // Don't show tooltip until container bounds are measured + if ( containerBounds.width === 0 || containerBounds.height === 0 ) { + return; + } + + // Use clientX/Y and subtract containerBounds to cancel out any stale offset. + // TooltipInPortal calculates: tooltipLeft + containerBounds.left + scrollX + // By passing (clientX - containerBounds.left), we get: + // (clientX - containerBounds.left) + containerBounds.left + scrollX = clientX + scrollX + // This gives correct page coordinates regardless of stale bounds. + showTooltip( { + tooltipData: arc.data, + tooltipLeft: event.clientX - containerBounds.left + tooltipOffsetX, + tooltipTop: event.clientY - containerBounds.top + tooltipOffsetY, + } ); + }; + + const pathProps: SVGProps< SVGPathElement > & { 'data-testid'?: string } = { + d: pie.path( arc ) || '', + fill: accessors.fill( arc.data ), + 'data-testid': 'pie-segment', + }; + + const groupProps: SVGProps< SVGGElement > = {}; + if ( withTooltips ) { + groupProps.onMouseMove = handleMouseMove; + groupProps.onMouseLeave = onMouseLeave; } - // Use clientX/Y and subtract containerBounds to cancel out any stale offset. - // TooltipInPortal calculates: tooltipLeft + containerBounds.left + scrollX - // By passing (clientX - containerBounds.left), we get: - // (clientX - containerBounds.left) + containerBounds.left + scrollX = clientX + scrollX - // This gives correct page coordinates regardless of stale bounds. - showTooltip( { - tooltipData: arc.data, - tooltipLeft: event.clientX - containerBounds.left + tooltipOffsetX, - tooltipTop: event.clientY - containerBounds.top + tooltipOffsetY, - } ); - }; - - const pathProps: SVGProps< SVGPathElement > & { 'data-testid'?: string } = { - d: pie.path( arc ) || '', - fill: accessors.fill( arc.data ), - 'data-testid': 'pie-segment', - }; - - const groupProps: SVGProps< SVGGElement > = {}; - if ( withTooltips ) { - groupProps.onMouseMove = handleMouseMove; - groupProps.onMouseLeave = onMouseLeave; - } - - // Estimate text width more accurately for background sizing - const fontSize = 12; - const estimatedTextWidth = getStringWidth( arc.data.label, { fontSize } ); - const labelPadding = 6; - const backgroundWidth = estimatedTextWidth + labelPadding * 2; - const backgroundHeight = fontSize + labelPadding * 2; - - return ( - - - { showLabels && hasSpaceForLabel && ( - - { providerTheme.labelBackgroundColor && ( - + + { showLabels && hasSpaceForLabel && ( + + { providerTheme.labelBackgroundColor && ( + + ) } + - ) } - - { arc.data.label } - - - ) } - - ); - } ); - } } - - ) } - - { /* Render SVG children (like Group, Text) inside the SVG */ } - { ! allSegmentsHidden && svgChildren } - - - - { showLegend && ( - - ) } + > + { arc.data.label } + + + ) } + + ); + } ); + } } + + ) } + + { /* Render SVG children (like Group, Text) inside the SVG */ } + { ! allSegmentsHidden && svgChildren } + + +
+ + { legendPosition === 'bottom' && legendElement } { withTooltips && tooltipOpen && tooltipData && ( @@ -472,7 +504,7 @@ const PieChartInternal = ( { { /* Render other React children for backward compatibility */ } { otherChildren } - +
); }; diff --git a/projects/js-packages/charts/src/charts/pie-chart/stories/donut.stories.tsx b/projects/js-packages/charts/src/charts/pie-chart/stories/donut.stories.tsx index 1c36cb686c3..61d90e17c2d 100644 --- a/projects/js-packages/charts/src/charts/pie-chart/stories/donut.stories.tsx +++ b/projects/js-packages/charts/src/charts/pie-chart/stories/donut.stories.tsx @@ -90,10 +90,8 @@ type Story = StoryObj< StoryArgs >; export const Default: Story = { args: { ...sharedThemeArgs, - size: 400, containerWidth: '432px', containerHeight: '432px', - resize: 'none', thickness: 0.5, gapScale: 0.03, cornerScale: 0.03, @@ -112,6 +110,25 @@ export const Default: Story = { }, }; +export const WithSize: Story = { + args: { + ...Default.args, + size: 200, + thickness: 0.3, + showLabels: false, + children: ( + + + User Activity + + + Total: 100K Users + + + ), + }, +}; + export const WithoutCenter: Story = { args: { ...Default.args, @@ -124,12 +141,12 @@ export const ErrorStates: Story = {

Empty Data

- +

Single Value

@@ -159,50 +176,6 @@ export const Thin: Story = { }, }; -export const Doughnut: Story = { - args: { - ...Default.args, - thickness: 0.5, - gapScale: 0.03, - cornerScale: 0.03, - size: 600, - containerWidth: '632px', - containerHeight: '632px', - children: ( - - - 🍩 Doughnut - - - Three donuts for the price of one! - - - ), - }, - parameters: { - docs: { - description: { - story: 'Doughnut chart variant with the thickness set to 0.5 (50%).', - }, - }, - }, -}; - -export const WithTooltipsDoughnut: Story = { - args: { - ...Default.args, - thickness: 0.5, - withTooltips: true, - }, - parameters: { - docs: { - description: { - story: 'Doughnut chart with interactive tooltips that appear on hover.', - }, - }, - }, -}; - export const Animation: Story = { args: { ...Default.args, @@ -220,69 +193,32 @@ export const WithLegend: Story = { export const WithCompositionLegend: Story = { render: args => ( -
-
-

Traditional Props-based

- - - - User Stats - - - 100K Total - - - -
-
-

Composition API

- - - - User Stats - - - 100K Total - - - - -
-
+ + + User Stats + + + 100K Total + + + +
), args: { data, thickness: 0.5, - containerHeight: '500px', }, argTypes: { legendInteractive: { @@ -302,40 +238,35 @@ export const WithCompositionLegend: Story = { export const InteractiveLegend: Story = { render: args => ( -
-

Interactive Donut Chart

-

+ + + + User Stats + + + 100K Total + + +

Click legend items to show/hide segments. The total value updates dynamically.

- - - - User Stats - - - 100K Total - - - -
+
), args: { data, - size: 400, thickness: 0.5, - containerHeight: '600px', }, parameters: { docs: { @@ -455,7 +386,8 @@ export const CustomLegend: Story = { ), args: { ...Default.args, - data: customerRevenueData.map( segment => ( { ...segment, label: '' } ) ), + data: customerRevenueData, + showLabels: false, thickness: 0.3, cornerScale: 0.03, gapScale: 0.01, diff --git a/projects/js-packages/charts/src/charts/pie-chart/stories/index.api.mdx b/projects/js-packages/charts/src/charts/pie-chart/stories/index.api.mdx index 4644e160710..e9d4d49c761 100644 --- a/projects/js-packages/charts/src/charts/pie-chart/stories/index.api.mdx +++ b/projects/js-packages/charts/src/charts/pie-chart/stories/index.api.mdx @@ -13,7 +13,10 @@ Main component for rendering pie and donut charts. | Prop | Type | Default | Description | | ---- | ---- | ------- | ----------- | | `data` | `DataPointPercentage[]` | - | **Required.** Array of data objects with label, value, and percentage | -| `size` | `number` | - | Diameter of the chart in pixels (omit for responsive behavior) | +| `width` | `number` | - | Width of the chart container in pixels. When omitted, fills parent width | +| `height` | `number` | - | Height of the chart container in pixels. When omitted, fills parent height | +| `size` | `number` | - | Maximum diameter of the pie in pixels. The pie shrinks if the container is smaller. When omitted, fills available space | +| `gap` | `GapSize` | `'md'` | Gap between chart elements (SVG, legend, children). Uses WordPress design system tokens | | `thickness` | `number` | `1` | Thickness of the pie chart segments (0-1). Values less than 1 create donut charts | | `padding` | `number` | `20` | Padding around the chart in pixels | | `gapScale` | `number` | `0` | Scale of gaps between segments (0-1) | diff --git a/projects/js-packages/charts/src/charts/pie-chart/stories/index.docs.mdx b/projects/js-packages/charts/src/charts/pie-chart/stories/index.docs.mdx index ff48de7e467..1b9642479c8 100644 --- a/projects/js-packages/charts/src/charts/pie-chart/stories/index.docs.mdx +++ b/projects/js-packages/charts/src/charts/pie-chart/stories/index.docs.mdx @@ -41,7 +41,6 @@ The simplest pie chart requires only a `data` prop with percentage-based data: - {/* HTML elements rendered outside the SVG */} - -

Chart Title

-
- - {/* SVG elements rendered inside the chart */} - - - Center Text - - - - {/* More HTML elements */} - - -

Additional information

-
-` } + {/* HTML elements rendered outside the SVG */} + +

Chart Title

+
+ + {/* SVG elements rendered inside the chart */} + + + Center Text + + + + {/* More HTML elements */} + + +

Additional information

+
+ ` } /> ### Benefits of Composition API @@ -128,27 +129,31 @@ Enable interactive tooltips that display data information on hover: ### Responsive Behavior -The Pie Chart component supports responsive sizing by omitting the `size` prop: - - +By default, charts **fill their parent container's dimensions**. The parent must have an explicit height: + code={ `// Fill parent container (default) - parent needs explicit height +
+ +
- // Fixed size - ` } + // Fixed dimensions + ` } +/> + +Use the `size` prop to constrain the maximum pie diameter. The pie will shrink if the container is smaller, but won't grow beyond this value: + + +` } /> +For more details on responsive behavior, see the [Responsive Design section](./?path=/docs/js-packages-charts-library-introduction--docs#responsive-design) in the introduction. + ### Data Validation and Error Handling The component includes built-in data validation with helpful error states: diff --git a/projects/js-packages/charts/src/charts/pie-chart/stories/index.stories.tsx b/projects/js-packages/charts/src/charts/pie-chart/stories/index.stories.tsx index dd2e9ba7364..c9781cd8914 100644 --- a/projects/js-packages/charts/src/charts/pie-chart/stories/index.stories.tsx +++ b/projects/js-packages/charts/src/charts/pie-chart/stories/index.stories.tsx @@ -35,7 +35,8 @@ const meta: Meta< StoryArgs > = { step: 10, default: 400, }, - description: 'Diameter of the pie chart in pixels', + description: + 'Maximum diameter of the pie in pixels. The pie shrinks if the container is smaller. When omitted, fills available space.', table: { category: 'Dimensions' }, }, thickness: { @@ -125,13 +126,26 @@ export const Default: Story = { cornerScale: 0, withTooltips: false, data, - resize: 'none', - size: 400, containerWidth: '432px', containerHeight: '432px', }, }; +export const WithSize: Story = { + args: { + ...Default.args, + size: 200, + }, +}; + +export const FixedDimensions: Story = { + args: { + ...Default.args, + width: 300, + height: 300, + }, +}; + export const Animation: Story = { args: { ...Default.args, @@ -157,51 +171,23 @@ export const WithLegend: Story = { args: { ...Default.args, showLegend: true, - containerHeight: '500px', }, }; export const WithCompositionLegend: Story = { render: args => ( -
-
-

Traditional Props-based Legend

- -
-
-

Composition API with Legend Component

- - - -
-
+ + + ), args: { data, - containerHeight: '500px', }, argTypes: { legendInteractive: { @@ -221,30 +207,27 @@ export const WithCompositionLegend: Story = { export const InteractiveLegend: Story = { render: args => ( -
-

Interactive Legend

-

+ +

Click legend items to show/hide segments. Percentages recalculate automatically for visible segments.

- -
+
), args: { data, size: 400, - containerHeight: '600px', }, parameters: { docs: { @@ -290,8 +273,7 @@ export const CustomLegendPositioning: Story = { legendShape: 'circle', size: 400, containerWidth: '432px', - containerHeight: '480px', - resize: 'none', + containerHeight: '432px', }, parameters: { docs: { @@ -303,19 +285,6 @@ export const CustomLegendPositioning: Story = { }, }; -const responsiveArgs = { ...Default.args, resize: 'both' as const }; -delete responsiveArgs.size; -export const Responsiveness: Story = { - args: responsiveArgs, - parameters: { - docs: { - description: { - story: 'Pie chart with responsive behavior. Uses size prop instead of width/height.', - }, - }, - }, -}; - export const CompositionAPI: Story = { render: args => { const chartData = args.data || [ @@ -437,7 +406,6 @@ export const CustomLabelColors: Story = { labelTextColor: '#FFFFFF', // White text for contrast against dark background labelBackgroundColor: 'rgba(0, 0, 0, 0.75)', // Dark semi-transparent background size: 400, - containerHeight: '500px', }, parameters: { docs: { @@ -461,12 +429,11 @@ export const ErrorStates: Story = {

Empty Data

- +

Invalid Percentage Total

Negative Values

Single Data Point

- +
), + args: { + containerHeight: '600px', + }, parameters: { docs: { description: { diff --git a/projects/js-packages/charts/src/charts/pie-chart/stories/tooltip.stories.tsx b/projects/js-packages/charts/src/charts/pie-chart/stories/tooltip.stories.tsx index 763033820fd..856f5dbfa27 100644 --- a/projects/js-packages/charts/src/charts/pie-chart/stories/tooltip.stories.tsx +++ b/projects/js-packages/charts/src/charts/pie-chart/stories/tooltip.stories.tsx @@ -45,11 +45,9 @@ const Template: StoryFn< typeof PieChart > = args => ; const tooltipStoryArgs = { ...sharedThemeArgs, data, - size: 400, withTooltips: true, containerWidth: '432px', containerHeight: '432px', - resize: 'none' as const, }; export const Default: StoryObj< typeof PieChart > = Template.bind( {} ); @@ -261,13 +259,13 @@ export const TooltipOffset: StoryObj< typeof PieChart > = { >

Default Offset (0, -15)

- +

Custom Offset (20, -30)

diff --git a/projects/js-packages/charts/src/charts/pie-semi-circle-chart/pie-semi-circle-chart.tsx b/projects/js-packages/charts/src/charts/pie-semi-circle-chart/pie-semi-circle-chart.tsx index 82276f39200..46c9d62764a 100644 --- a/projects/js-packages/charts/src/charts/pie-semi-circle-chart/pie-semi-circle-chart.tsx +++ b/projects/js-packages/charts/src/charts/pie-semi-circle-chart/pie-semi-circle-chart.tsx @@ -7,7 +7,7 @@ import clsx from 'clsx'; import { useCallback, useContext, useMemo } from 'react'; import { Legend, useChartLegendItems } from '../../components/legend'; import { BaseTooltip } from '../../components/tooltip'; -import { useElementHeight, useInteractiveLegendData, usePrefersReducedMotion } from '../../hooks'; +import { useElementSize, useInteractiveLegendData, usePrefersReducedMotion } from '../../hooks'; import { GlobalChartsProvider, useChartId, @@ -181,7 +181,7 @@ const PieSemiCircleChartInternal: FC< PieSemiCircleChartProps > = ( { renderTooltip = renderDefaultPieSemiCircleTooltip, } ) => { const chartId = useChartId( providedChartId ); - const [ legendRef, legendHeight ] = useElementHeight< HTMLDivElement >(); + const [ legendRef, , legendHeight ] = useElementSize< HTMLDivElement >(); const { tooltipOpen, tooltipLeft, tooltipTop, tooltipData, hideTooltip, showTooltip } = useTooltip< DataPointPercentage >(); diff --git a/projects/js-packages/charts/src/charts/private/with-responsive/test/with-responsive.test.tsx b/projects/js-packages/charts/src/charts/private/with-responsive/test/with-responsive.test.tsx index 89006214341..fedf762944f 100644 --- a/projects/js-packages/charts/src/charts/private/with-responsive/test/with-responsive.test.tsx +++ b/projects/js-packages/charts/src/charts/private/with-responsive/test/with-responsive.test.tsx @@ -47,10 +47,10 @@ describe( 'withResponsive', () => { expect( component ).toHaveStyle( { width: '400px' } ); } ); - test( 'passes size prop equal to container width', () => { - render( ); + test( 'passes explicit size prop through to component', () => { + render( ); const component = screen.getByTestId( 'mock-component' ); - expect( component ).toHaveAttribute( 'data-size', '600' ); + expect( component ).toHaveAttribute( 'data-size', '200' ); } ); } ); @@ -61,8 +61,8 @@ describe( 'withResponsive', () => { expect( wrapper ).toHaveStyle( { width: '100%', height: '100%' } ); } ); - test( 'wrapper uses size prop for dimensions when provided', () => { - render( ); + test( 'wrapper uses explicit width/height for dimensions when provided', () => { + render( ); const wrapper = screen.getByTestId( 'responsive-wrapper' ); expect( wrapper ).toHaveStyle( { width: '200px', height: '200px' } ); } ); diff --git a/projects/js-packages/charts/src/charts/private/with-responsive/with-responsive.tsx b/projects/js-packages/charts/src/charts/private/with-responsive/with-responsive.tsx index 94587ac8ccb..e0ddc489066 100644 --- a/projects/js-packages/charts/src/charts/private/with-responsive/with-responsive.tsx +++ b/projects/js-packages/charts/src/charts/private/with-responsive/with-responsive.tsx @@ -86,10 +86,11 @@ export function withResponsive< T extends Exclude< BaseChartProps< unknown >, 'o aspectRatio, } ); - // Use measured dimensions, but fall back to explicit props if measurement returns 0 - // (e.g., during initial render or in test environments without DOM measurement) - const effectiveWidth = measuredWidth || size || width || 0; - const effectiveHeight = measuredHeight || size || height || 0; + // Use measured dimensions, but fall back to explicit width/height props if measurement returns 0 + // (e.g., during initial render or in test environments without DOM measurement). + // Do not use size here — size controls chart element dimensions (e.g. pie diameter), not container dimensions. + const effectiveWidth = measuredWidth || width || 0; + const effectiveHeight = measuredHeight || height || 0; const defaultHeight = hasAspectRatio ? 'auto' : '100%'; @@ -99,14 +100,14 @@ export function withResponsive< T extends Exclude< BaseChartProps< unknown >, 'o data-testid="responsive-wrapper" className={ styles.container } style={ { - width: size ?? width ?? '100%', - height: size ?? height ?? defaultHeight, + width: width ?? '100%', + height: height ?? defaultHeight, } } >
diff --git a/projects/js-packages/charts/src/hooks/index.ts b/projects/js-packages/charts/src/hooks/index.ts index a0e7ebdf730..6bf6c934c6e 100644 --- a/projects/js-packages/charts/src/hooks/index.ts +++ b/projects/js-packages/charts/src/hooks/index.ts @@ -3,7 +3,7 @@ export { useChartMouseHandler } from './use-chart-mouse-handler'; export { useXYChartTheme } from './use-xychart-theme'; export { useChartDataTransform } from './use-chart-data-transform'; export { useChartMargin } from './use-chart-margin'; -export { useElementHeight } from './use-element-height'; +export { useElementSize } from './use-element-size'; export { useHasLegendChild } from './use-has-legend-child'; export { useTextTruncation } from './use-text-truncation'; export { useZeroValueDisplay } from './use-zero-value-display'; diff --git a/projects/js-packages/charts/src/hooks/test/use-element-height.test.tsx b/projects/js-packages/charts/src/hooks/test/use-element-size.test.tsx similarity index 66% rename from projects/js-packages/charts/src/hooks/test/use-element-height.test.tsx rename to projects/js-packages/charts/src/hooks/test/use-element-size.test.tsx index 61571a981d7..d8c1315d7fd 100644 --- a/projects/js-packages/charts/src/hooks/test/use-element-height.test.tsx +++ b/projects/js-packages/charts/src/hooks/test/use-element-size.test.tsx @@ -1,5 +1,5 @@ import { renderHook, waitFor } from '@testing-library/react'; -import { useElementHeight } from '../use-element-height'; +import { useElementSize } from '../use-element-size'; // Mock ResizeObserver class MockResizeObserver { @@ -25,7 +25,7 @@ class MockResizeObserver { // Store original ResizeObserver const originalResizeObserver = globalThis.ResizeObserver; -describe( 'useElementHeight', () => { +describe( 'useElementSize', () => { let mockResizeObserver; beforeEach( () => { @@ -39,75 +39,82 @@ describe( 'useElementHeight', () => { jest.clearAllMocks(); } ); - it( 'should return initial height of 0 by default', () => { - const { result } = renderHook( () => useElementHeight() ); - const [ refCallback, height ] = result.current; + it( 'should return initial dimensions of 0 by default', () => { + const { result } = renderHook( () => useElementSize() ); + const [ refCallback, width, height ] = result.current; + expect( width ).toBe( 0 ); expect( height ).toBe( 0 ); expect( typeof refCallback ).toBe( 'function' ); } ); - it( 'should return custom initial height when provided', () => { - const { result } = renderHook( () => useElementHeight( { initialHeight: 100 } ) ); - const [ , height ] = result.current; + it( 'should return custom initial dimensions when provided', () => { + const { result } = renderHook( () => + useElementSize( { initialWidth: 200, initialHeight: 100 } ) + ); + const [ , width, height ] = result.current; + expect( width ).toBe( 200 ); expect( height ).toBe( 100 ); } ); - it( 'should update height when element is attached', async () => { + it( 'should update dimensions when element is attached', async () => { const mockElement = { - getBoundingClientRect: jest.fn( () => ( { height: 150 } ) ), + getBoundingClientRect: jest.fn( () => ( { width: 300, height: 150 } ) ), }; - const { result } = renderHook( () => useElementHeight() ); + const { result } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; // Attach the element refCallback( mockElement as unknown as HTMLDivElement ); await waitFor( () => { - expect( result.current[ 1 ] ).toBe( 150 ); + expect( result.current[ 1 ] ).toBe( 300 ); + expect( result.current[ 2 ] ).toBe( 150 ); } ); expect( mockElement.getBoundingClientRect ).toHaveBeenCalled(); } ); - it( 'should handle element with zero height', async () => { + it( 'should handle element with zero dimensions', async () => { const mockElement = { - getBoundingClientRect: jest.fn( () => ( { height: 0 } ) ), + getBoundingClientRect: jest.fn( () => ( { width: 0, height: 0 } ) ), }; - const { result } = renderHook( () => useElementHeight() ); + const { result } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; refCallback( mockElement as unknown as HTMLDivElement ); await waitFor( () => { expect( result.current[ 1 ] ).toBe( 0 ); + expect( result.current[ 2 ] ).toBe( 0 ); } ); } ); - it( 'should handle getBoundingClientRect returning undefined height', async () => { + it( 'should handle getBoundingClientRect returning undefined dimensions', async () => { const mockElement = { - getBoundingClientRect: jest.fn( () => ( { height: undefined } ) ), + getBoundingClientRect: jest.fn( () => ( { width: undefined, height: undefined } ) ), }; - const { result } = renderHook( () => useElementHeight() ); + const { result } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; refCallback( mockElement as unknown as HTMLDivElement ); await waitFor( () => { expect( result.current[ 1 ] ).toBe( 0 ); + expect( result.current[ 2 ] ).toBe( 0 ); } ); } ); it( 'should disconnect previous observer when new element is attached', () => { const mockElement1 = { - getBoundingClientRect: jest.fn( () => ( { height: 100 } ) ), + getBoundingClientRect: jest.fn( () => ( { width: 100, height: 100 } ) ), }; const mockElement2 = { - getBoundingClientRect: jest.fn( () => ( { height: 200 } ) ), + getBoundingClientRect: jest.fn( () => ( { width: 200, height: 200 } ) ), }; const disconnectSpy = jest.fn(); @@ -119,7 +126,7 @@ describe( 'useElementHeight', () => { jest.spyOn( globalThis, 'ResizeObserver' ).mockImplementation( () => mockObserver ); - const { result } = renderHook( () => useElementHeight() ); + const { result } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; // Attach first element @@ -133,7 +140,7 @@ describe( 'useElementHeight', () => { it( 'should disconnect observer when element is removed (null)', () => { const mockElement = { - getBoundingClientRect: jest.fn( () => ( { height: 100 } ) ), + getBoundingClientRect: jest.fn( () => ( { width: 100, height: 100 } ) ), }; const disconnectSpy = jest.fn(); @@ -145,7 +152,7 @@ describe( 'useElementHeight', () => { jest.spyOn( globalThis, 'ResizeObserver' ).mockImplementation( () => mockObserver ); - const { result } = renderHook( () => useElementHeight() ); + const { result } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; // Attach element @@ -159,7 +166,7 @@ describe( 'useElementHeight', () => { it( 'should create ResizeObserver and observe element', () => { const mockElement = { - getBoundingClientRect: jest.fn( () => ( { height: 100 } ) ), + getBoundingClientRect: jest.fn( () => ( { width: 100, height: 100 } ) ), }; const observeSpy = jest.fn(); @@ -171,7 +178,7 @@ describe( 'useElementHeight', () => { jest.spyOn( globalThis, 'ResizeObserver' ).mockImplementation( () => mockObserver ); - const { result } = renderHook( () => useElementHeight() ); + const { result } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; refCallback( mockElement as unknown as HTMLDivElement ); @@ -181,7 +188,7 @@ describe( 'useElementHeight', () => { } ); it( 'should maintain stable refCallback reference across re-renders', () => { - const { result, rerender } = renderHook( () => useElementHeight() ); + const { result, rerender } = renderHook( () => useElementSize() ); const firstRefCallback = result.current[ 0 ]; @@ -195,26 +202,27 @@ describe( 'useElementHeight', () => { it( 'should work with different element types', async () => { const mockSpanElement = { - getBoundingClientRect: jest.fn( () => ( { height: 50 } ) ), + getBoundingClientRect: jest.fn( () => ( { width: 75, height: 50 } ) ), }; - const { result } = renderHook( () => useElementHeight() ); + const { result } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; refCallback( mockSpanElement as unknown as HTMLDivElement ); await waitFor( () => { - expect( result.current[ 1 ] ).toBe( 50 ); + expect( result.current[ 1 ] ).toBe( 75 ); + expect( result.current[ 2 ] ).toBe( 50 ); } ); } ); - it( 'should update height when ResizeObserver callback is triggered', async () => { + it( 'should update dimensions when ResizeObserver callback is triggered', async () => { let resizeCallback; const mockElement = { getBoundingClientRect: jest .fn() - .mockReturnValueOnce( { height: 100 } ) - .mockReturnValueOnce( { height: 200 } ), + .mockReturnValueOnce( { width: 100, height: 100 } ) + .mockReturnValueOnce( { width: 200, height: 150 } ), }; jest.spyOn( globalThis, 'ResizeObserver' ).mockImplementation( callback => { @@ -226,13 +234,14 @@ describe( 'useElementHeight', () => { }; } ); - const { result } = renderHook( () => useElementHeight() ); + const { result } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; refCallback( mockElement as unknown as HTMLDivElement ); await waitFor( () => { expect( result.current[ 1 ] ).toBe( 100 ); + expect( result.current[ 2 ] ).toBe( 100 ); } ); // Simulate resize @@ -240,6 +249,7 @@ describe( 'useElementHeight', () => { await waitFor( () => { expect( result.current[ 1 ] ).toBe( 200 ); + expect( result.current[ 2 ] ).toBe( 150 ); } ); expect( mockElement.getBoundingClientRect ).toHaveBeenCalledTimes( 2 ); @@ -247,7 +257,7 @@ describe( 'useElementHeight', () => { it( 'should handle unmount properly', () => { const mockElement = { - getBoundingClientRect: jest.fn( () => ( { height: 100 } ) ), + getBoundingClientRect: jest.fn( () => ( { width: 100, height: 100 } ) ), }; const disconnectSpy = jest.fn(); @@ -257,7 +267,7 @@ describe( 'useElementHeight', () => { unobserve: jest.fn(), } ) ); - const { result, unmount } = renderHook( () => useElementHeight() ); + const { result, unmount } = renderHook( () => useElementSize() ); const [ refCallback ] = result.current; refCallback( mockElement as unknown as HTMLDivElement ); diff --git a/projects/js-packages/charts/src/hooks/use-element-height.ts b/projects/js-packages/charts/src/hooks/use-element-size.ts similarity index 51% rename from projects/js-packages/charts/src/hooks/use-element-height.ts rename to projects/js-packages/charts/src/hooks/use-element-size.ts index e4e34168412..9fea24ffed2 100644 --- a/projects/js-packages/charts/src/hooks/use-element-height.ts +++ b/projects/js-packages/charts/src/hooks/use-element-size.ts @@ -1,19 +1,23 @@ import { useState, useCallback, useRef } from 'react'; /** - * Hook to measure the height of a DOM element. - * Returns a ref to attach to the element and the current height in pixels. + * Hook to measure the width and height of a DOM element. + * Returns a ref callback to attach to the element and the current dimensions in pixels. * * @param {object} props - Optional props. + * @param {number} props.initialWidth - The initial width to use. * @param {number} props.initialHeight - The initial height to use. * - * @return {[Function, number]} A tuple containing a ref to attach to the element and the current height in pixels + * @return {[Function, number, number]} A tuple containing a ref callback, width, and height in pixels */ -export function useElementHeight< T extends HTMLElement = HTMLDivElement >( { +export function useElementSize< T extends HTMLElement = HTMLDivElement >( { + initialWidth = 0, initialHeight = 0, }: { + initialWidth?: number; initialHeight?: number; -} = {} ): [ ( node: T | null ) => void, number ] { +} = {} ): [ ( node: T | null ) => void, number, number ] { + const [ width, setWidth ] = useState( initialWidth ); const [ height, setHeight ] = useState( initialHeight ); const observerRef = useRef< ResizeObserver | null >( null ); @@ -24,7 +28,9 @@ export function useElementHeight< T extends HTMLElement = HTMLDivElement >( { } if ( node ) { const handleResize = () => { - setHeight( node.getBoundingClientRect().height || 0 ); + const rect = node.getBoundingClientRect(); + setWidth( rect.width || 0 ); + setHeight( rect.height || 0 ); }; handleResize(); const resizeObserver = new window.ResizeObserver( handleResize ); @@ -33,5 +39,5 @@ export function useElementHeight< T extends HTMLElement = HTMLDivElement >( { } }, [] ); - return [ refCallback, height ]; + return [ refCallback, width, height ]; } diff --git a/projects/js-packages/charts/src/types.ts b/projects/js-packages/charts/src/types.ts index 1737d569279..781f064b134 100644 --- a/projects/js-packages/charts/src/types.ts +++ b/projects/js-packages/charts/src/types.ts @@ -364,15 +364,17 @@ export type BaseChartProps< T = DataPoint | DataPointDate | LeaderboardEntry > = */ className?: string; /** - * Width of the chart in pixels + * Width of the chart container in pixels. When omitted, the chart fills its parent's width. */ width?: number; /** - * Height of the chart in pixels + * Height of the chart container in pixels. When omitted, the chart fills its parent's height. */ height?: number; /** - * Size of the chart in pixels for pie and donut charts + * Maximum diameter of the pie in pixels (pie and donut charts only). + * The pie will shrink if the container is smaller than this value. + * When omitted, the pie fills the available space. */ size?: number; /**