[charts-pro] Hide values outside minStart and maxEnd in zoom slider #18336
Conversation
|
Deploy preview: https://deploy-preview-18336--material-ui-x.netlify.app/ Updated pages: Bundle size reportTotal Size Change: ▼-9.31KB(-0.07%) - Total Gzip Change: ▼-2.94KB(-0.08%) Show details for 100 more bundles (22 more not shown)@mui/x-charts-pro parsed: ▼-3.57KB(-1.05%) gzip: ▼-1.46KB(-1.40%) |
bernardobelchior
added
type: enhancement
CodSpeed Performance ReportMerging #18336 will not alter performanceComparing Summary
|
| ); | ||
| } | ||
|
|
||
| export function calculateZoomFromPointImpl( |
There was a problem hiding this comment.
This separation makes it easier to unit test.
There was a problem hiding this comment.
Did you also consider creating calculatePointFromZoomImpl() to simplify the computation in ChartAxisZoomSliderActiveTrack.tsx and have the conversion logic zoom <-> SVG in a single place
There was a problem hiding this comment.
I don't think I understood your question. calculatePointFromZoom, which uses calculatePointFromZoomImpl is already being used in ChartAxisZoomSliderActiveTrack. Where exactly do you think it would make sense to add it?
I think we should, but I think that's a different feature because it should also apply to the zoom data itself, not just visually to the slider, right? If the zoom is uncontrolled, it should never break any constraint. Then there's the edge case where constraints are invalid: minSpan is 100 and minStart is 10. In that case we should throw in development. In production, I'm not sure. Maybe we should ignore minSpan in that case? If the zoom is controlled, then we need to decide. My opinion is that we should always meet the constraints. If you have minStart 10 and provide a zoom start of 5, then you'll receive an onChange where zoom start is 10. I think this might be good because we can get controlled data from a URL search param which the end user can manipulate but we don't want the developer to have to worry about out of range values. However, we can also consider accepting what the user defines. In the URL search param case from above, the developer would need to clamp the values manually. @alexfauquette @JCQuintas what do you think? |
|
I tend to think that a control logic should be as dumb as possible. The dev provides something => you apply it. Otherwise, when they try to do something custom, they must handle our internal logic (and its potential future modifications). If the zoom constraint got modified the |
|
|
||
| The zoom slider uses the same limits as the zooming options. You can set the `minStart`, `maxEnd`, `minSpan`, and `maxSpan` properties on the axis config to restrict the zoom slider range. | ||
|
|
||
| Values outside the `minStart` and `maxEnd` range will not be displayed in the zoom slider. |
There was a problem hiding this comment.
| Values outside the `minStart` and `maxEnd` range will not be displayed in the zoom slider. | |
| The `minStart` and `maxEnd` define the extremum of the zoom slider. |
| Values outside the `minStart` and `maxEnd` range will not be displayed in the zoom slider. | |
| The zoom slider does not display values outside the `minStart` and `maxEnd` range. |
| ); | ||
| } | ||
|
|
||
| export function calculateZoomFromPointImpl( |
There was a problem hiding this comment.
Did you also consider creating calculatePointFromZoomImpl() to simplify the computation in ChartAxisZoomSliderActiveTrack.tsx and have the conversion logic zoom <-> SVG in a single place
|
I agree with @alexfauquette that the user-controlled one should be fairly dumb. On the uncontrolled mode, maybe the minStart/maxEnd should have priority over the Though they could do that by having the zoom controlled 🤔 |
Hide values outside minStart and maxEnd in zoom slider, as suggested by @alexfauquette here.
Screen.Recording.2025-06-11.at.17.57.35.mov