fix: unnecessary line breaks in Bar labels#6214
Conversation
29b8ed3 to
404fbd5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6214 +/- ##
==========================================
+ Coverage 96.70% 96.71% +0.01%
==========================================
Files 221 221
Lines 19880 19886 +6
Branches 4102 4103 +1
==========================================
+ Hits 19224 19233 +9
+ Misses 650 647 -3
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @ei, thanks for the PR. I have another change in progress that has a potential to break this again. May I please ask you to add a VR test or storybook that shows this behaviour? Or more direct unit tests? The coverage now only expects that the viewbox is provided, doesn't assert the size of the label. Thanks |
|
Hi @PavelVanecek , Regards, |
1cb0c42 to
041467e
Compare
|
Thanks! We have visual regression tests on storybooks but we usually get rate limited on those. Unit or playwright test won't have this problem. |
Pass the parent viewbox so the right label size can be calculated. Fixes recharts#6072
041467e to
e5bae8b
Compare
Bundle ReportChanges will increase total bundle size by 154 bytes (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-cjsAssets Changed:
|
ckifer
left a comment
There was a problem hiding this comment.
Is this an issue in other charts too?
|
It's not specific to BarChart, so using a Bar in a ComposedChart would give the same issue. |
|
Right, but how about the other types of charts - Line, Area, Scatter, Pie, etc.? |
|
Haven't noticed the issue with other types I've used |
|
Sounds good thanks! |
Description
When creating Labels for a bar, pass the parent's viewbox, so that the label sizes can be well calculated.
Related Issue
#6072
Motivation and Context
There was an issue where bar labels either overflowed the chart's bounds, or on the opposite were constrained to a too small space. This was because as the parentViewBox was not available, the labels bounds were wrongly calculated (expecially for labels outside the bars)
How Has This Been Tested?
Tested Those changes with different BarChart configurations; in all cases the labels seem to take the space that's expected (see example screenshots).
Types of changes
Checklist: