fix: don't filter zero out of brush scaleValues#6323
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6323 +/- ##
==========================================
+ Coverage 96.61% 96.63% +0.01%
==========================================
Files 221 221
Lines 20329 20329
Branches 4170 4173 +3
==========================================
+ Hits 19641 19645 +4
+ Misses 681 677 -4
Partials 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 35 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the Brush component where zero values were being filtered out from scaleValues, causing index misalignment when the first scale value is 0. The fix changes the filter condition from Boolean to explicitly checking for null/undefined values.
- Changed filter logic in Brush component to preserve zero values
- Added comprehensive test coverage for the zero value edge case
- Added a storybook story to demonstrate the fix
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/cartesian/Brush.tsx | Updated filter condition to preserve zero values in scaleValues |
| test/cartesian/Brush.spec.tsx | Added test case for zero value handling in brush navigation |
| storybook/stories/Examples/cartesian/Brush/Brush.stories.tsx | Added NoChartBrush story for visual testing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| .domain() | ||
| .map(entry => prevScale(entry)) | ||
| .filter(Boolean); | ||
| .filter(value => value != null); |
There was a problem hiding this comment.
[nitpick] Consider using strict equality (!== null && value !== undefined) instead of loose equality (!= null) for more explicit null/undefined checking, which is a common TypeScript best practice.
| .filter(value => value != null); | |
| .filter(value => value !== null && value !== undefined); |
Description
we were filtering out
0which breaks Brush when the firstscaleValuein the list happens to be 0. This usually works because our default margin adds value to the scale values to make the first one > 0Related Issue
#6308
Motivation and Context
bug fix
How Has This Been Tested?
unit test, storybook
Screenshots (if appropriate):
Types of changes
Checklist: