[charts] Fix wheel zoom not centering on cursor position#22811
Merged
Conversation
`rafThrottle(onWheelRef.current)` captured the first-render `onWheel` closure, locking in the initial (pre-layout, zero) `drawingArea` and `optionsLookup`. The cursor-to-anchor ratio was then computed from stale dimensions, so wheel zoom ignored the cursor position. Wrap the call in an indirection so the throttled function always invokes the latest closure, matching the pinch and drag gesture hooks. Regression from mui#22708.
JCQuintas
added
scope: charts
Deploy previewBundle size
PerformanceTotal duration: 1,889.13 ms -88.24 ms(-4.5%) | Renders: 67 (+0)
23 tests within noise — details Metric alarms
Check out the code infra dashboard for more information about this PR. |
Renders a responsive ScatterChartPro (no explicit width/height) so the drawing area is empty on first render and only measured afterwards, reproducing the stale-closure bug. Zooming near the left edge keeps the leftmost tick, near the right edge keeps the rightmost; a stale drawing area anchors at the same place regardless of cursor.
JCQuintas
enabled auto-merge (squash)
June 15, 2026 12:45
alexfauquette
approved these changes
Jun 15, 2026
Comment on lines
+117
to
+125
| // Regression test for https://github.com/mui/mui-x/pull/22811 | ||
| // Wheel zoom must anchor on the cursor position. With a responsive chart the | ||
| // drawing area is empty on the first render and only gets its real size once | ||
| // the container is measured. The wheel handler must read that up-to-date | ||
| // drawing area; using a stale (empty) one makes the zoom ignore the cursor. | ||
| // Zooming near the left edge keeps the leftmost tick visible, while zooming | ||
| // near the right edge keeps the rightmost one. | ||
| // `width`/`height` are intentionally omitted so the chart sizes itself from | ||
| // the wrapper, reproducing the empty-then-measured drawing area. |
Member
There was a problem hiding this comment.
Might not need that many comment
JCQuintas
disabled auto-merge
June 15, 2026 13:39
JCQuintas
commented
Jun 15, 2026
Co-authored-by: Jose C Quintas Jr <[email protected]> Signed-off-by: Jose C Quintas Jr <[email protected]>
JCQuintas
enabled auto-merge (squash)
June 15, 2026 13:43
JCQuintas
disabled auto-merge
June 15, 2026 14:54
mbrookes
pushed a commit
to mbrookes/mui-x
that referenced
this pull request
Jun 27, 2026
Signed-off-by: Jose C Quintas Jr <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Wheel zoom no longer centers on the cursor position — it behaves as if zooming around a fixed/incorrect anchor.
Cause
In
useWheelGesture, the throttled handler was created as:rafThrottlecaptures theonWheelRef.currentvalue at effect-mount time, i.e. the first-renderonWheelclosure. That closure holds the first-renderdrawingArea(zero/default before layout has measured) andoptionsLookup, and is never refreshed. The cursor-to-anchor ratio(point.x - left) / widthis then computed from stale dimensions, so the zoom ignores the cursor.The
onWheelRefindirection exists precisely to avoid this staleness, but passing.currentdefeated it.Fix
Wrap the call in an indirection so the throttled function always invokes the latest closure — matching the existing
usePinchGesture,useDragGesture, anduseDragOnPressGesturehooks, which already read.currentinside the throttled callback.Regression
Introduced in #22708 (decouple interaction hook from the cartesian zoom). Before that refactor the throttled callback was rebuilt each effect run with fresh
drawingAreadeps, so cursor anchoring worked.