Add shrink wrap for bar chart when overflow
This pull request is to resolve #1952.
This should allow the user to set a property to make the first and last bars touch the sides of the container when they would otherwise overflow and get auto-spaced (in spaceEvenly()).
When shrinkWrapOnWidthOverflow is set to true, the first and last bars in the chart should be touching the sides instead of having space at the start/end.
The bars should still be spaced evenly.
Checklist
- [X] Unit tests pass
- [X] Manually tested
- [X] Formatted correctly
- [X] Doc comments
Screenshots:
With shrinkWrapOnWidthOverflow = true:
With shrinkWrapOnWidthOverflow = false:
I have pushed changes to address the Copilot code review 😊
Does it matter what type of Alignment we're using for BarChartAlignment?
Or it just works in all the cases?
Hi 👋
I have added the property into props, as requested.
As for your question:
The shrinkWrapOnWidthOverflow: true only applies when the items would overflow the chart area, which causes the spaceEvenly function to get called.
This case happens when BarChartAlignment is start, end, or center and the total width of all items and spacing is greater than the chart width (tempX > viewWidth), or when it's set to spaceEvenly. When BarChartAlignment is spaceAround or spaceBetween, the spacing is calculated differently than the spaceEvenly function.
This can be seen in lib/src/extensions/bar_chart_data_extension.dart.
Please let me know if there are any other issues 😊
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:warning: Please upload report for BASE (main@be5b449). Learn more about missing BASE report.
:warning: Report is 9 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #1953 +/- ##
=======================================
Coverage ? 92.52%
=======================================
Files ? 50
Lines ? 3717
Branches ? 0
=======================================
Hits ? 3439
Misses ? 278
Partials ? 0
| Flag | Coverage Δ | |
|---|---|---|
| flutter | 92.52% <100.00%> (?) |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
I added a test that should make for 100% coverage now 😊
To be honest, I don't see this flag as the fix to the bug that happens when you define start, end or center with a spacing that leads to overflow.
I think when it happens, we should render them edge-to-edge (just like when shrinkWrapOnWidthOverflow is true in your implementation). So we simply say if the required width is more than the available width, we can set spaceBetween (under the hood).
Or maybe we can define an overflowFallbackAlignment that is spaceEvenly() by default and users can change it to spaceBetween().
What do you think?
Yeah, that makes sense if it just automatically applies if there would be overflow, rather than setting the property manually.
My thought was that some people might not want the bars to touch the edge so having a property to alter the behaviour when there is an overflow accounted for that.
But for ease-of-use, I think you're right it's better to let it get worked out behind the scenes when there is an overflow, and users can calculate the appropriate spacing based on their container size if they want to avoid the overflow behaviour.
I will update it 😊
Is that what you had in mind? 😊
Hi, @imaNNeo 👋 I didn't get a response from you in a few months, so I was wondering if this PR is complete or needs to be updated again. Please let me know! Thank you 😊
Hey, sorry for my delayed reply.
I see that spaceEvenly functionality is broken now.
Can you please fix it and also give me a reproducible code for the main issue that you initially had?
Then I can check the functionalities!
| ❌ spaceEvenly | ✅ spaceBetween | ✅ spaceAround | |
|---|---|---|---|
| before | |||
| after |