[charts-pro] Fix zoom filtering adjusting axis too soon #18992
Conversation
|
Deploy preview: https://deploy-preview-18992--material-ui-x.netlify.app/ Bundle size report
|
| if (axis.scaleType === 'point' || axis.scaleType === 'band' || typeof val === 'string') { | ||
| return dataIndex >= minVal && dataIndex <= maxVal; | ||
| } | ||
| if (typeof val === 'string') { |
There was a problem hiding this comment.
I wonder if we can remove this from continuous scales 🤔
I kept it because I didn't want to cause a breaking change, but it doesn't make sense for val to be a string here if we're using a continuous scale (which only supports numeric values).
If we can remove this, it should simplify this method since we'll no longer need to depend on axisData.
There was a problem hiding this comment.
i don't see why we would leave it here. It was probably added as a way to tell typescript to stop complaining about the last return, since checking axis.scaleType doesn't typecheck val
There was a problem hiding this comment.
Yeah, makes sense. I separated the refactor into this PR so we can focus on the fix here.
That should allow me to test this use case more thoroughly without worrying whether the culprit is the refactor or the fix
CodSpeed Performance ReportMerging #18992 will not alter performanceComparing Summary
Footnotes |
| const minVal = (zoomStart * max) / 100; | ||
| const maxVal = (zoomEnd * max) / 100; |
There was a problem hiding this comment.
Since we are considering discret value, what about keeping every thing as before (accept equality) and just rounding the min/max
| const minVal = (zoomStart * max) / 100; | |
| const maxVal = (zoomEnd * max) / 100; | |
| const minVal = Math.floor((zoomStart * max) / 100); | |
| const maxVal = Math.ceil((zoomEnd * max) / 100); |
There was a problem hiding this comment.
Sorry, I didn't get what you mean. What are you suggesting?
There was a problem hiding this comment.
Your current solution is about doing the math in a continuous space, going from the first tick (the 0) to the last tick (the data.length) and then you do comarison with array indexes thanks to the -1
My proposal was to only consider indexes and manage extremes by rounding up and down
There was a problem hiding this comment.
Sure, makes sense 👍
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
d799570 to
5d3773b
Compare
5d3773b to
ca4ae74
Compare
8c2159d to
8a48188
Compare
Fix #18991.
Before:
Screen.Recording.2025-07-30.at.17.20.56.mov
After:
Screen.Recording.2025-07-30.at.17.27.24.mov