Skip to content

Charts: Fix chart height and size calculations for pie and donut charts#47074

Open
adamwoodnz wants to merge 9 commits intotrunkfrom
charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and
Open

Charts: Fix chart height and size calculations for pie and donut charts#47074
adamwoodnz wants to merge 9 commits intotrunkfrom
charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and

Conversation

@adamwoodnz
Copy link
Contributor

@adamwoodnz adamwoodnz commented Feb 12, 2026 •
edited
Loading

Fixes CHARTS-164

Overview

The size prop on pie and donut charts was pulling double duty — controlling both the container dimensions (via withResponsive) and the pie diameter. This caused incorrect sizing in responsive contexts: charts would either overflow their container or fail to fill available space, because the responsive wrapper and the pie itself were fighting over the same value.

This PR separates those concerns. size now only constrains the maximum pie diameter, while width/height control the container. A new SVG wrapper element measures available space so the pie can shrink to fit or fill its container automatically. The underlying useElementHeight hook is generalized to useElementSize to support these two-dimensional measurements across all chart types.

Proposed changes:

  • Generalized useElementHeight hook to useElementSize, adding width measurement alongside height. This enables components to measure both dimensions of their container.
  • Decoupled pie chart size from container dimensions — the size prop now controls only the pie diameter (not the responsive wrapper), and the pie auto-sizes to fit available space when no size is specified.
  • Replaced the pie chart's manual flex layout with a Stack component from @wordpress/ui for proper gap control and column layout.
  • Added an SVG wrapper element so the pie chart can measure available space and shrink responsively within flex containers.
  • Added a gap prop to PieChart for controlling spacing between chart elements (SVG, legend, children) using WordPress design system tokens.
  • Updated the responsive wrapper (withResponsive) to stop using size for container dimensions — size is now passed through as a chart-level prop only.
  • Removed the explicit design-tokens CSS import from the pie chart — loading global CSS is the responsibility of the consuming application, not individual components.
  • Fixed donut story color indexing bug where blanking labels with label: '' caused findIndex to always return 0, giving all segments the same color. Uses showLabels: false instead.
  • Updated stories to reflect the new responsive sizing behaviour and removed stale stories/args.
  • Updated all tests to cover the new useElementSize return signature [ref, width, height].

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

  1. Run the Charts Storybook: cd projects/js-packages/charts && pnpm run storybook
  2. Navigate to the Pie Chart stories
  3. Verify the Default story renders a pie that fills its container responsively (no explicit size prop)
  4. Verify the WithSize story constrains the pie to 200px diameter
  5. Verify the WithLegend story shows the legend below the pie with proper spacing and the pie shrinks to accommodate it
  6. Verify the InteractiveLegend story allows toggling segments via legend clicks
  7. Navigate to the Donut Chart stories and repeat 3 - 6
  8. Check the Bar Chart, Line Chart and Semi Circle Pie Chart default stories still render correctly (they now use useElementSize)
  9. Resize the browser window and verify all charts respond to container size changes

Linear Issue: CHARTS-164

Made with Cursor

@adamwoodnz adamwoodnz added [Type] Bug When a feature is broken and / or not performing as intended [Status] In Progress RNA [JS Package] Charts labels Feb 12, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2026 •
edited
Loading

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions
Copy link
Contributor

github-actions bot commented Feb 12, 2026

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

@adamwoodnz adamwoodnz self-assigned this Feb 12, 2026
@adamwoodnz adamwoodnz marked this pull request as draft February 12, 2026 01:55
@jp-launch-control
Copy link

jp-launch-control bot commented Feb 12, 2026 •
edited
Loading

Code Coverage Summary

Coverage changed in 1 file.

File Coverage Δ% Δ Uncovered
projects/js-packages/charts/src/charts/pie-chart/pie-chart.tsx 82/85 (96.47%) 0.17% 0 💚

1 file is newly checked for coverage.

File Coverage
projects/js-packages/charts/src/hooks/use-element-size.ts 17/17 (100.00%) 💚

Full summary · PHP report · JS report

Base automatically changed from cursor/CHARTS-166-responsive-wrapper-shrinking-65a8 to trunk February 12, 2026 19:58
@adamwoodnz adamwoodnz force-pushed the charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and branch from de190fa to 049e63e Compare February 13, 2026 02:20
adamwoodnz and others added 6 commits February 18, 2026 16:41
@adamwoodnz @cursoragent
refactor(charts): Generalize useElementHeight to useElementSize
4711fa7 
Pie and semi-circle charts need both width and height to compute
correct chart dimensions; the previous hook only exposed height.
Renaming to useElementSize and returning [ref, width, height] lets
callers use either or both dimensions (bar/line/semi-circle skip
width via destructuring; pie chart uses both for size calculations).

Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz @cursoragent
fix(charts): Decouple pie size from container dimensions
417d51c 
The `size` prop was being used for both container dimensions and pie
diameter, causing incorrect sizing in responsive contexts. The pie
would overflow or not fill its container because `withResponsive`
set width/height from `size`, conflating two distinct concerns.

Now `size` only constrains the maximum pie diameter while container
dimensions come from width/height props or responsive measurement.
The pie shrinks to fit when the container is smaller than `size`,
and fills available space when `size` is omitted.

Replace the outer div with a WordPress Stack component for proper
gap control via design tokens, and position the legend through
conditional rendering instead of CSS flex-direction reversal.

Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz @cursoragent
fix(charts): Remove explicit design-tokens CSS import
c17d933 
Loading global CSS like design-tokens.css is the responsibility
of the consuming application, not individual chart components.

Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz @cursoragent
docs(charts): Update pie chart size/dimension docs
a264012 
The size prop no longer controls container dimensions — it
constrains the maximum pie diameter. Width and height now
control the container. Updated JSDoc, API reference, stories
argTypes, and the docs MDX to reflect the new behavior and
remove the broken Responsiveness story reference.

Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz @cursoragent
fix(charts): Update tooltip stories to use height instead of size
3d085a6 
The tooltip stories still referenced the old `size` prop and set
`resize: 'none'`, both of which are no longer needed after decoupling
pie size from container dimensions. Switches to `height` and lets
container sizing handle the rest.

Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz @cursoragent
fix(charts): Fix donut story color mapping and clean up stories
43245a3 
Blanking labels with `label: ''` broke pie chart color indexing —
`findIndex` by label always returned 0, so all segments got the same
color. Use original data with `showLabels: false` instead.

Also removes redundant Doughnut/WithTooltipsDoughnut stories,
simplifies composition and interactive legend examples, and
switches ErrorStates from `size` to `height`.

Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz adamwoodnz force-pushed the charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and branch from f7358d9 to 43245a3 Compare February 18, 2026 03:41
@adamwoodnz adamwoodnz changed the title Charts: Fix chart height and size calculations for pie chart and variants Charts: Fix chart height and size calculations for pie and donut charts Feb 18, 2026
adamwoodnz and others added 2 commits February 18, 2026 16:53
@adamwoodnz @cursoragent
test(charts): Update tests for useElementHeight → useElementSize rename
aba64dc 
The hook was renamed and now returns [ref, width, height] instead of
[ref, height]. Update mocks in bar-chart and line-chart tests to use
the new module path and 3-tuple return value.

Also update with-responsive tests: size no longer controls wrapper
dimensions (only width/height props do), and size is passed through
directly rather than derived from container width.

Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz @cursoragent
test(charts): Add WithSize story for donut chart
ba6c129 
Demonstrates the explicit `size` prop on a donut chart with a thin
ring, hidden labels, and centered text — useful for verifying the
fixed-size rendering path decoupled from container dimensions.

Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz adamwoodnz marked this pull request as ready for review February 18, 2026 04:08
Copilot AI review requested due to automatic review settings February 18, 2026 04:08
@adamwoodnz adamwoodnz added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Feb 18, 2026
Copilot AI reviewed Feb 18, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors chart sizing and responsive behavior to fix issues with pie and donut chart height calculations. It generalizes the useElementHeight hook to useElementSize for measuring both width and height, and decouples the size prop from container dimensions so it now specifically controls the maximum pie diameter.

Changes:

  • Renamed and enhanced useElementHeight to useElementSize to measure both width and height of DOM elements
  • Refactored pie chart layout to use WordPress UI Stack component and measure available space dynamically
  • Updated size prop semantics: now controls maximum chart element dimensions (pie diameter) rather than container dimensions, with container controlled by width/height props

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
projects/js-packages/charts/src/hooks/use-element-size.ts Renamed from use-element-height.ts and enhanced to measure both width and height, returning a 3-value tuple
projects/js-packages/charts/src/hooks/test/use-element-size.test.tsx Updated all tests to handle the new hook signature with width and height
projects/js-packages/charts/src/hooks/index.ts Updated export to use new useElementSize name
projects/js-packages/charts/src/types.ts Updated prop documentation to clarify that size controls chart element dimensions (pie diameter), not container
projects/js-packages/charts/src/charts/private/with-responsive/with-responsive.tsx Removed size from container dimension calculations - now only used as chart-level prop
projects/js-packages/charts/src/charts/private/with-responsive/test/with-responsive.test.tsx Updated tests to reflect new size prop behavior
projects/js-packages/charts/src/charts/pie-chart/pie-chart.tsx Refactored to use Stack component, measure available space with svg-wrapper, and calculate pie size from measured dimensions
projects/js-packages/charts/src/charts/pie-chart/pie-chart.module.scss Added svg-wrapper styles with flex properties and removed manual gap/legend-top classes
projects/js-packages/charts/src/charts/pie-semi-circle-chart/pie-semi-circle-chart.tsx Updated to use useElementSize with 3-value destructuring
projects/js-packages/charts/src/charts/line-chart/line-chart.tsx Updated to use useElementSize with 3-value destructuring
projects/js-packages/charts/src/charts/line-chart/test/line-chart.test.tsx Updated mock to return 3 values for useElementSize
projects/js-packages/charts/src/charts/bar-chart/bar-chart.tsx Updated to use useElementSize with 3-value destructuring
projects/js-packages/charts/src/charts/bar-chart/test/bar-chart.test.tsx Updated mock to return 3 values for useElementSize
projects/js-packages/charts/src/charts/pie-chart/stories/index.stories.tsx Updated stories to demonstrate new sizing behavior, added WithSize and FixedDimensions stories, removed Responsiveness story
projects/js-packages/charts/src/charts/pie-chart/stories/index.docs.mdx Updated documentation to explain new size prop semantics and responsive behavior
projects/js-packages/charts/src/charts/pie-chart/stories/index.api.mdx Updated API documentation to clarify size, width, height props and added gap prop
projects/js-packages/charts/src/charts/pie-chart/stories/tooltip.stories.tsx Updated tooltip stories to use height instead of size for container dimensions
projects/js-packages/charts/src/charts/pie-chart/stories/donut.stories.tsx Updated donut stories, removed redundant stories, simplified composition examples
projects/js-packages/charts/changelog/charts-164-fix-chart-height-and-size-calculations-for-pie-chart-and Added changelog entry describing the fix

@adamwoodnz @cursoragent
fix(charts): Remove redundant size prop from ErrorStates stories
20c012b 
After decoupling `size` from container dimensions, the ErrorStates
examples were still passing `size={300}` alongside `height={300}`,
which is redundant — `size` constrains the pie diameter while
`height` constrains the container, and both set to 300 is
effectively a no-op for `size`.

Drop `size` from all four error-state examples so they size
responsively, keeping only `height={300}` on Single Data Point
where a fixed container dimension is useful.

Refs: #47074 (comment)
Co-authored-by: Cursor <cursoragent@cursor.com>
@adamwoodnz adamwoodnz requested a review from a team February 18, 2026 04:50
@adamwoodnz adamwoodnz requested a review from a team February 18, 2026 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@adamwoodnz adamwoodnz

Labels

[JS Package] Charts RNA [Status] Needs Review This PR is ready for review. [Tests] Includes Tests [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@adamwoodnz