-
Notifications
You must be signed in to change notification settings - Fork 658
Chart: prevent unexpected page scrolling on zoom (T1314606) #31866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chart: prevent unexpected page scrolling on zoom (T1314606) #31866
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue (T1314606) where unexpected page scrolling occurs when performing zoom operations on a chart using the mouse wheel. The fix introduces a timeout-based mechanism to prevent default scroll behavior for a brief window after zoom interactions, even when the zoom limit has been reached.
Key Changes:
- Introduced a 500ms prevention window that continues to block page scrolling after zoom operations
- Modified the event handling logic to track and extend the prevention window on subsequent wheel events
- Added comprehensive test coverage for the timeout-based scroll prevention behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts | Implements timer-based scroll prevention logic with new lastWheelTimer variable, setLastWheelTimer() function, and modified preventDefaults() signature; exports SCROLL_PREVENTION_TIMEOUT constant |
| packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js | Adds test case verifying that scroll prevention continues for 500ms after zoom limit is reached, testing timer reset on subsequent wheel events within the window |
Comments suppressed due to low confidence (1)
packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts:573
- The
lastWheelTimershould be cleared in thecleanup()function to prevent potential memory leaks when the chart is disposed. AddclearTimeout(lastWheelTimer);at the beginning of thecleanup()function.
cleanup() {
renderer.root.off(EVENTS_NS);
// @ts-expect-error
zoomAndPan.actionData?.rect?.dispose();
// @ts-expect-error
zoomAndPan.actionData = null;
renderer.root.css({ 'touch-action': '' });
},
packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js
Show resolved
Hide resolved
packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js
Outdated
Show resolved
Hide resolved
packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js
Outdated
Show resolved
Hide resolved
b0ae4d3 to
8f51c84
Compare
8f51c84 to
e325bbe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
packages/devextreme/js/__internal/viz/chart_components/zoom_and_pan.ts:573
- The
lastWheelTimershould be cleared in thecleanup()function to prevent potential memory leaks. When the zoom and pan functionality is disposed or reconfigured, the timer may still be active. AddclearTimeout(lastWheelTimer);at the beginning of the cleanup function, similar to how other timers are handled in the codebase (e.g.,tracker.ts).
cleanup() {
renderer.root.off(EVENTS_NS);
// @ts-expect-error
zoomAndPan.actionData?.rect?.dispose();
// @ts-expect-error
zoomAndPan.actionData = null;
renderer.root.css({ 'touch-action': '' });
},
packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js
Show resolved
Hide resolved
packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js
Show resolved
Hide resolved
packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js
Show resolved
Hide resolved
packages/devextreme/testing/tests/DevExpress.viz.charts/zoomAndPan.tests.js
Show resolved
Hide resolved
| init() { | ||
| const chart = this; | ||
| const renderer = this._renderer; | ||
| let lastWheelTimer: number | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor]
| let lastWheelTimer: number | undefined; | |
| let lastWheelTimer?: ReturnType<typeof setTimeout>; |
No description provided.