[charts-pro] Zoom pointer improvements #17000

JCQuintas · 2025-03-17T13:51:46Z

Main change

Use use-gesture to recognise the gestures. Right now the lib is a bit slow to have updates, but it has some solid behaviour detection.

Create a interaction listener plugin. This allows us to register events directly in the SVG.

The current implementation migrates most of the interactions to the schema below.

Other changes

Add zoom and pan tests for BarChartPro, LineChartPro, and ScatterChartPro
- Currently we can't properly test the pinch interaction. It seems RTL and use-gesture can't communicate properly there.
- I've tried manual events as well (fireEvent/element.dispatchEvent), but couldn't figure out a way to make it work.
Divided the zoom behaviours into hooks implementing the usePanOnDrag, useZoomOnPinch and useZoomOnWheel hooks

…events-improvement

bernardobelchior

The new implementation seems to simplify the logic a lot! Great job!

I still don't understand some parts of the interaction listener hook, so left a few questions

...src/internals/plugins/corePlugins/useChartInteractionListener/useChartInteractionListener.ts

bernardobelchior · 2025-03-20T08:38:50Z

...src/internals/plugins/corePlugins/useChartInteractionListener/useChartInteractionListener.ts

+  const addMultipleInteractionListeners: AddMultipleInteractionListeners = React.useCallback(
+    (interactions, callback) => {
+      const cleanups = interactions.map((interaction) =>
+        // @ts-expect-error Overriding the type because the type of the callback is not inferred
+        addInteractionListener(interaction, callback),
+      );
+      return {
+        cleanup: () => cleanups.forEach((cleanup) => cleanup.cleanup()),
+      };
+    },
+    [addInteractionListener],
+  );


Not sure if this would work, but probably worth a shot:

Suggested change

const addMultipleInteractionListeners: AddMultipleInteractionListeners = React.useCallback(

(interactions, callback) => {

const cleanups = interactions.map((interaction) =>

// @ts-expect-error Overriding the type because the type of the callback is not inferred

addInteractionListener(interaction, callback),

);

return {

cleanup: () => cleanups.forEach((cleanup) => cleanup.cleanup()),

};

},

[addInteractionListener],

);

const addMultipleInteractionListeners = React.useCallback<AddMultipleInteractionListeners>(

(interactions, callback) => {

const cleanups = interactions.map((interaction) =>

addInteractionListener(interaction, callback),

);

return {

cleanup: () => cleanups.forEach((cleanup) => cleanup.cleanup()),

};

},

[addInteractionListener],

);

That resolves to the same type, so it doesn't work.

packages/x-charts/src/internals/plugins/featurePlugins/useChartVoronoi/useChartVoronoi.ts

packages/x-charts/tsconfig.json

bernardobelchior · 2025-03-20T08:52:08Z

Did a small test on iOS Safari and it seems to me that zooming in is too slow. Not in the sense of performance, but we might need to make it zoom a bit faster.

In desktop it's working perfectly 👍

JCQuintas · 2025-03-20T09:21:51Z

Did a small test on iOS Safari and it seems to me that zooming in is too slow. Not in the sense of performance, but we might need to make it zoom a bit faster.

In desktop it's working perfectly 👍

Thanks for checking, will debug

alexfauquette

You have a bug with controlled zoom. The dragging get accelerated.

I could not figure why by just reading the code

Capture.video.du.2025-03-20.10-54-05.mp4

alexfauquette · 2025-03-20T08:28:41Z

packages/x-charts-pro/src/BarChartPro/BarChartPro.zoom.test.tsx

+    // scroll, we scroll exactly in the center of the svg
+    // And we do it 200 times which is the lowest number to trigger a zoom where both A and D are not visible
+    for (let i = 0; i < 200; i += 1) {
+      fireEvent.wheel(svg, { deltaY: -1, clientX: 50, clientY: 50 });
+    }


It's surprising that it can not be done with delatY: -200

alexfauquette · 2025-03-20T08:29:50Z

packages/x-charts-pro/src/BarChartPro/BarChartPro.zoom.test.tsx

+    // scroll back
+    for (let i = 0; i < 200; i += 1) {
+      fireEvent.wheel(svg, { deltaY: 1, clientX: 50, clientY: 50 });
+    }


If as you said 200 is the limit to only see B and C, then just a few scroll back should be enough

alexfauquette · 2025-03-20T08:44:00Z

packages/x-charts-pro/src/LineChartPro/LineChartPro.zoom.test.tsx

+  const options = {
+    wrapper: ({ children }: { children?: React.ReactNode }) => (
+      <div style={{ width: 100, height: 130 }}>{children}</div>
+    ),
+  };


What is the advantage of the wrapper compared to just providing the width/height to the chart props?

packages/x-charts-pro/src/internals/plugins/useChartProZoom/gestureHooks/usePanOnDrag.ts

alexfauquette · 2025-03-20T09:22:42Z

packages/x-charts/src/ChartsTooltip/utils.tsx

+    // Drag event is triggered by mobile touch or mouse drag.
+    const positionOnDragHandler = instance.addMultipleInteractionListeners(
+      ['move', 'drag'],


It looks like a hack to have the use-gesture whereas the pointermove event is exactly what we are looking for

Seems like useGesture also support native events
https://use-gesture.netlify.app/docs/gestures/#native-event-handlers-in-react

In this case it is probably better to use pointerMove indeed

...ternals/plugins/corePlugins/useChartInteractionListener/useChartInteractionListener.types.ts

alexfauquette · 2025-03-20T09:47:00Z

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

+      cleanInteractionHandler.cleanup();
+      setInteractionHandler.cleanup();


Effectively cleaner 👍

alexfauquette · 2025-03-20T09:48:31Z

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

@@ -155,13 +158,15 @@ export const useChartCartesianAxis: ChartPlugin<UseChartCartesianAxisSignature<a
      return () => {};
    }

-    const handleMouseClick = (event: MouseEvent) => {
-      event.preventDefault();
+    const axisClickHandler = instance.addInteractionListener('drag', (state) => {


Same feeling about forcing the usage of use-gesture events for stuff they are not planned for

Drag is meant to detect tap/click as well.

github-actions · 2025-03-21T09:14:52Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

…events-improvement

alexfauquette · 2025-03-25T16:48:51Z

docs/data/charts/zoom-and-pan/ExternalZoomManagement.tsx

        xAxis={[
          {
            zoom: true,


If you'r lookig for what triggers the re-render. I'ts probably the xAxis

github-actions · 2025-03-28T10:40:47Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

JCQuintas added 30 commits March 6, 2025 19:15

add plugin start

b03ccf2

add chart interaction using @use-gesture/react

6d80993

replace tooltip logic

ba8b61a

rename vars

db75765

update axis event click handler

04f4a3a

improve

7f6b43e

rename

baccbf1

prevent useInteractionItemProps re-renders

b4f0d24

Fix perf related to react spring

48b461e

Merge commit 'd3cde4cfcbc42b685da49928caefa531ea3c3711' into pointer-…

e38fcee

…events-improvement

fix missing

7988ff0

add mouse position tracker back

29923fa

migrate polar axis

6abb966

add wheel listeners

7ebb091

Merge commit '06d8025ed7b5eb14b4d125924b4eeefea9af12a2' into pointer-…

637e193

…events-improvement

log

6dad5f2

fix listeners bug

6a8ad69

clear animation on drag

989c284

move zoom wheel setup to its own hook

d9b51cd

zoom on pinch

1cb800a

move pan on drag

a019178

cleanup cache logic

3c18086

fix mobile pinch

61bf657

reorder

9595fac

fix mobile interaction

223c9b1

allow config multiple listeners at once

568f7da

tree shake

c89dabd

remove el check

01899c7

use memo for storing data

f9a56a7

remove comment

b93cbd1

JCQuintas added 5 commits March 18, 2025 12:09

Merge commit 'bb6ce455efdd7abaee82eed64a9dbef8584abb23' into pointer-…

8873928

…events-improvement

add bar tests

0dece29

add further tests

f595e0c

remove only

290e7f0

Merge branch 'master' into pointer-events-improvement

75872f4

JCQuintas marked this pull request as ready for review March 19, 2025 21:41

JCQuintas requested review from alexfauquette and bernardobelchior March 19, 2025 21:41

JCQuintas added 2 commits March 19, 2025 23:34

fix wheel update depth issue

ecafae6

remove timeout on wheel need

c5f6c8e

bernardobelchior reviewed Mar 20, 2025

View reviewed changes

JCQuintas added 2 commits March 20, 2025 10:18

add docs

cc9e738

rework tap

0edea79

alexfauquette reviewed Mar 20, 2025

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 21, 2025

JCQuintas force-pushed the pointer-events-improvement branch from 76ab4f5 to 0edea79 Compare March 21, 2025 14:03

JCQuintas added 5 commits March 21, 2025 19:00

allow pointermove

fd88fbe

move utils

02c1a29

use native events

1cccafa

pan

c55a5e3

Merge commit '83b5b33ec34e1cb6b91f134c0c442b3ac9da3cf1' into pointer-…

f1a0e33

…events-improvement

alexfauquette reviewed Mar 25, 2025

View reviewed changes

ensure zoom is called on RAF

220efd1

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 25, 2025

Add debouce and automatic interacting

7fb1449

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Mar 28, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[charts-pro] Zoom pointer improvements #17000

[charts-pro] Zoom pointer improvements #17000

JCQuintas commented Mar 17, 2025 •

edited

Loading

bernardobelchior left a comment

bernardobelchior Mar 20, 2025

JCQuintas Mar 20, 2025

bernardobelchior commented Mar 20, 2025 •

edited

Loading

JCQuintas commented Mar 20, 2025

alexfauquette left a comment

alexfauquette Mar 20, 2025

alexfauquette Mar 20, 2025

alexfauquette Mar 20, 2025

alexfauquette Mar 20, 2025

alexfauquette Mar 20, 2025

JCQuintas Mar 21, 2025

alexfauquette Mar 20, 2025

alexfauquette Mar 20, 2025

JCQuintas Mar 21, 2025

github-actions bot commented Mar 21, 2025

alexfauquette Mar 25, 2025

github-actions bot commented Mar 28, 2025

		cleanInteractionHandler.cleanup();
		setInteractionHandler.cleanup();

[charts-pro] Zoom pointer improvements #17000

Are you sure you want to change the base?

[charts-pro] Zoom pointer improvements #17000

Conversation

JCQuintas commented Mar 17, 2025 • edited Loading

Main change

Other changes

bernardobelchior left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernardobelchior commented Mar 20, 2025 • edited Loading

JCQuintas commented Mar 20, 2025

alexfauquette left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 21, 2025

Choose a reason for hiding this comment

github-actions bot commented Mar 28, 2025

JCQuintas commented Mar 17, 2025 •

edited

Loading

bernardobelchior commented Mar 20, 2025 •

edited

Loading