[charts] Fix zoom discard inconsistency #19535

bernardobelchior · 2025-09-11T14:13:30Z

Alternative to #19332.

mui-bot · 2025-09-11T14:15:26Z

Deploy preview: https://deploy-preview-19535--material-ui-x.netlify.app/

Bundle size report

Bundle	Parsed size	Gzip size
@mui/x-data-grid	0B^(0.00%)	0B^(0.00%)
@mui/x-data-grid-pro	0B^(0.00%)	▼-1B^(0.00%)
@mui/x-data-grid-premium	0B^(0.00%)	🔺+1B^(0.00%)
@mui/x-charts	🔺+1.31KB^(+0.42%)	🔺+340B^(+0.35%)
@mui/x-charts-pro	🔺+2.01KB^(+0.50%)	🔺+446B^(+0.35%)
@mui/x-date-pickers	0B^(0.00%)	0B^(0.00%)
@mui/x-date-pickers-pro	0B^(0.00%)	0B^(0.00%)
@mui/x-tree-view	0B^(0.00%)	0B^(0.00%)
@mui/x-tree-view-pro	0B^(0.00%)	🔺+1B^(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 9e06422

bernardobelchior · 2025-09-11T15:40:56Z

...ages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/computeAxisValue.ts

          context: AxisValueFormatterContext<TScaleName>,
        ) => string),
-    };
+    } as ComputedAxis<ContinuousScaleName, any, ChartsAxisProps>;


Couldn't make this work without casting, unfortunately. satisfies didn't work either.

Yes, I assume it's because the scale type comes from a distinct object. So it TS cannot do the relation between the scaleType and the scale.

I don't think their is an easy workaround except the one you did previously by having some line of codes enforcing the scale and scaleType to be coherent like isBandScale(scale) && isBandScaleConfig(axis)

codspeed-hq · 2025-09-11T15:52:34Z

CodSpeed Performance Report

Merging #19535 will not alter performance

_{Comparing bernardobelchior:fix-zoom-discard-inconsistency (9e06422) with master (92e4dc2)¹}

Summary

✅ 10 untouched

No successful run was found on master (9f1a07f) during the generation of this report, so 92e4dc2 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

...ages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/computeAxisValue.ts

alexfauquette · 2025-09-15T08:00:30Z

...ages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/computeAxisValue.ts

          context: AxisValueFormatterContext<TScaleName>,
        ) => string),
-    };
+    } as ComputedAxis<ContinuousScaleName, any, ChartsAxisProps>;


Yes, I assume it's because the scale type comes from a distinct object. So it TS cannot do the relation between the scaleType and the scale.

I don't think their is an easy workaround except the one you did previously by having some line of codes enforcing the scale and scaleType to be coherent like isBandScale(scale) && isBandScaleConfig(axis)

alexfauquette · 2025-09-15T08:15:35Z

...ages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/computeAxisValue.ts

+        filter,
+      );
+      scale = scale.copy();
+      scale.domain([minData, maxData]);


This should be slightly more complex. The filters is here to remove some data points.

But it should still apply domainLimit strategies: nice(tickNumber) or the domainLimit(min, max) if defined.

We could add a test with a BarChart with series data set to [1, 4.5, 5] and x-axis zoom set to [0, 50] (ditch the last item)

domainLimit='nice': y-axis should go from 0 to 5

domainLimit='strict': y-axis should go from 0 to 4.5

Dark mode is the actual docs demo, light mode is this PR preview

Capture.video.du.2025-09-15.10-11-46.mp4

This will have a side effect impact: The domainLimit as a function could be called twice:

one when creating scales

one when applying the filter

Not a big deal for me

Should be fixed with the most recent commit 👍

JCQuintas · 2025-09-15T13:01:42Z

...ts/src/internals/plugins/featurePlugins/useChartCartesianAxis/createAxisFilterMapper.test.ts


 describe('createContinuousScaleGetAxisFilter', () => {
  describe('linear scale', () => {
+    const scale = getScale('linear', [0, 100], [0, 100]).nice();


All tests for createContinuousScaleGetAxisFilter use nice now.

We should also test the strict behaviour.

We should also test the strict behaviour.

What issues do you think testing the strict behavior will catch? As far as I can see, createContinuousScaleGetAxisFilter isn't affected by whether the domain is nice or not.

Ah true, I was still thinking of the previous behaviour 🤔

alexfauquette

You have an issue with preview

Capture.video.du.2025-09-15.18-20-50.mp4

bernardobelchior · 2025-09-15T17:12:39Z

You have an issue with preview

Capture.video.du.2025-09-15.18-20-50.mp4

Fixed and added a visual regression test 👍

alexfauquette · 2025-09-16T08:51:30Z

@bernardobelchior I fixed scritp issues, and added filterMode: 'discard', to your visual regression test such that it now test the two regression I spotted during the review

preview scalling like main chart
discard no applying the nice() method to scale

bernardobelchior added type: bug It doesn't behave as expected. scope: charts Changes related to the charts. labels Sep 11, 2025

bernardobelchior commented Sep 11, 2025

View reviewed changes

bernardobelchior added 10 commits September 11, 2025 17:56

[charts] Fix zoom discard inconsistency

bf6ee2a

Avoid filter extrema if not needed

a2d2da9

More refactors

8ba27fc

Fix rebase

0616d68

Make TS happy

ca53465

Simplify code further

74dfd2f

Fix more TS issues

bf69514

Fix tests

4222ff0

Fix lint and typescript

659454a

Fix preview

22732d7

bernardobelchior force-pushed the fix-zoom-discard-inconsistency branch from 12273a3 to 22732d7 Compare September 11, 2025 16:56

Fix lint

9820a86

bernardobelchior marked this pull request as ready for review September 11, 2025 17:26

bernardobelchior requested review from JCQuintas and alexfauquette as code owners September 11, 2025 17:26

alexfauquette reviewed Sep 15, 2025

View reviewed changes

Address PR feedback

297a054

bernardobelchior force-pushed the fix-zoom-discard-inconsistency branch from ac05e1e to 297a054 Compare September 15, 2025 10:21

bernardobelchior requested a review from alexfauquette September 15, 2025 10:59

Fix missing boolean

f3f5254

JCQuintas reviewed Sep 15, 2025

View reviewed changes

Fix regression

2678c5f

bernardobelchior requested a review from JCQuintas September 15, 2025 13:25

JCQuintas approved these changes Sep 15, 2025

View reviewed changes

alexfauquette requested changes Sep 15, 2025

View reviewed changes

bernardobelchior added 2 commits September 15, 2025 18:10

Add regression test

59489de

Fix zoom preview

0fc124f

bernardobelchior requested a review from alexfauquette September 15, 2025 17:12

scripts

1d9626a

alexfauquette approved these changes Sep 16, 2025

View reviewed changes

add filter discard mode

9e06422

bernardobelchior merged commit ea4d39f into mui:master Sep 17, 2025
22 checks passed

bernardobelchior deleted the fix-zoom-discard-inconsistency branch September 17, 2025 08:43

Uh oh!

Uh oh!

[charts] Fix zoom discard inconsistency #19535

[charts] Fix zoom discard inconsistency #19535

Uh oh!

Conversation

bernardobelchior commented Sep 11, 2025

Uh oh!

mui-bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle size report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codspeed-hq bot commented Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #19535 will not alter performance

Summary

Footnotes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JCQuintas Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexfauquette left a comment

Choose a reason for hiding this comment

Uh oh!

bernardobelchior commented Sep 15, 2025

Uh oh!

alexfauquette commented Sep 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mui-bot commented Sep 11, 2025 •

edited

Loading

codspeed-hq bot commented Sep 11, 2025 •

edited

Loading

JCQuintas Sep 15, 2025 •

edited

Loading