[charts] Use voronoi cells to trigger interaction with scatter items #10981

alexfauquette · 2023-11-09T16:53:39Z

Changelog

🚀 The scatter charts now use voronoi to trigger items

Users needed to hover the item to highlight the scatter item or show the tooltip.
Now they can interact with data by triggering the closest element. See the docs page for more info.

mui-bot · 2023-11-09T17:00:28Z

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

Updated pages:

docs/data/charts/scatter/scatter.md

Generated by 🚫 dangerJS against 1167199

github-actions · 2023-11-20T11:48:06Z

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

docs/data/charts/scatter/scatter.md

packages/x-charts/src/ChartsVoronoiHandler/ChartsVoronoiHandler.tsx

alexfauquette · 2023-12-04T11:04:53Z

packages/x-charts/src/ScatterChart/Scatter.tsx

@@ -94,7 +95,7 @@ function Scatter(props: ScatterProps) {
          key={dataPoint.id}
          cx={0}
          cy={0}
-          r={markerSize}
+          r={(dataPoint.isHighlighted ? 1.2 : 1) * markerSize}


Suggested change

r={(dataPoint.isHighlighted ? 1.2 : 1) * markerSize}

r={markerSize}

Note: Highcharts also do increase the point radius, when highlighted, but that is probably a separate story/functionality and would need to be controllable. 🤷 🙈

OK, so we agree it's interesting to discuss this point, but not for this PR :)

Oh yes, clearly. 👍

alexfauquette · 2023-12-04T11:07:32Z

packages/x-charts/src/context/InteractionProvider.tsx

@@ -71,6 +93,9 @@ const dataReducer: React.Reducer<Omit<InteractionState, 'dispatch'>, Interaction
      return { ...prevState, item: null };

    case 'updateAxis':
+      if (action.data.x === prevState.axis.x && action.data.y === prevState.axis.y) {


This update is not directly for voronoi. It's a way to "improve perf" If axis do not change, we do not create a new object to avoid triggering rerender at each mouse movement

alexfauquette · 2023-12-04T11:10:05Z

packages/x-charts/src/hooks/useAxisEvents.ts

@@ -5,6 +5,9 @@ import { SVGContext, DrawingContext } from '../context/DrawingProvider';
 import { isBandScale } from '../internals/isBandScale';
 import { AxisDefaultized } from '../models/axis';

+function getAsANumber(value: number | Date) {


Fix TS errors

Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

LukasTy

This is a major improvement, great work! 💯 💙
Leaving some nitpicks. 😉

docs/data/charts/scatter/scatter.md

docs/data/charts/scatter/VoronoiInteraction.tsx

docs/data/charts/scatter/scatter.md

LukasTy · 2023-12-07T11:30:05Z

packages/x-charts/src/ChartsVoronoiHandler/ChartsVoronoiHandler.tsx

+    // TODO: A perf optimisation of voronoi could be to use the last point as the intial point for the next search.
+    const handleMouseMove = (event: MouseEvent) => {
+      // Get mouse coordinate in global SVG space
+      const pt = svgRef.current!.createSVGPoint();


Nit: Have you considered using a full word (i.e.: point, svgPoint) for this variable?
Or do you have an argument as to why the shorthand form is better? 🤔

It's mostly because it's a trash variable. I create it to access the matrixTransform method and then it's over, So I avoid to pick a meaining full variable name

An option could be to create a helper getSVGPoint

const svgPoint = getSVGPoint(svgRef, event)

Gotcha. 👌
Yup, the suggested helper does sound nice. 👍

packages/x-charts/src/ChartsVoronoiHandler/ChartsVoronoiHandler.tsx

LukasTy · 2023-12-07T11:41:03Z

packages/x-charts/src/ChartsVoronoiHandler/ChartsVoronoiHandler.tsx

+  /**
+   * Defines the maximal distance between a scatter point and the pointer that triggers the interaction.
+   * If `undefined`, the radius is assumed to be infinite.
+   * @default undefined


Have you considered going with a specific default? 🤔
I checked that Highcharts also have the same behavior, but when exploring the demo, I felt that 25 seemed the most natural to me. 🤔 🤷
What do others think? @flaviendelangle @joserodolfofreitas?

Did not considered it because nivo don't even allow this

Okay, if it's only my opinion, we can disregard it. There is nothing wrong with this default. 👌
In that case, WDYT about aligning the demo with defaults?
It's what initially tripped me a bit because after seeing the demo I expected the default radius to be 50. 🙈

LukasTy · 2023-12-07T11:53:23Z

packages/x-charts/src/ScatterChart/Scatter.tsx

@@ -94,7 +95,7 @@ function Scatter(props: ScatterProps) {
          key={dataPoint.id}
          cx={0}
          cy={0}
-          r={markerSize}
+          r={(dataPoint.isHighlighted ? 1.2 : 1) * markerSize}


Note: Highcharts also do increase the point radius, when highlighted, but that is probably a separate story/functionality and would need to be controllable. 🤷 🙈

packages/x-charts/src/ScatterChart/ScatterChart.tsx

packages/x-charts/src/context/InteractionProvider.tsx

Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

POC delauney highlight

08343f8

alexfauquette marked this pull request as draft November 9, 2023 16:53

alexfauquette added the component: charts This is the name of the generic UI component, not the React module! label Nov 9, 2023

alexfauquette changed the title ~~[charts] Use voronoi cells to trigger interaction with scatter items~~ [WIP][charts] Use voronoi cells to trigger interaction with scatter items Nov 9, 2023

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

alexfauquette added 2 commits November 30, 2023 13:03

Merge remote-tracking branch 'upstream/next' into scatter-interaction

d90a3eb

perf inprovement

0a2e718

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

alexfauquette added 4 commits December 1, 2023 15:57

save rerender

6ed578a

proposal

a18286d

Merge remote-tracking branch 'upstream/next' into scatter-interaction

3345a3f

scripts

bff736b

alexfauquette force-pushed the scatter-interaction branch from 5ca11d6 to bff736b Compare December 1, 2023 17:16

alexfauquette commented Dec 4, 2023

View reviewed changes

alexfauquette and others added 2 commits December 4, 2023 12:12

Apply suggestions from code review

9c21ef0

Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

scripts

27d6b8b

alexfauquette changed the title ~~[WIP][charts] Use voronoi cells to trigger interaction with scatter items~~ [charts] Use voronoi cells to trigger interaction with scatter items Dec 4, 2023

alexfauquette marked this pull request as ready for review December 4, 2023 11:21

alexfauquette requested review from flaviendelangle and LukasTy December 4, 2023 13:51

LukasTy added the enhancement This is not a bug, nor a new feature label Dec 7, 2023

LukasTy approved these changes Dec 7, 2023

View reviewed changes

alexfauquette and others added 5 commits December 7, 2023 14:40

add helper getSVGPoint

e48d6bc

update demo

7c61a21

Apply suggestions from code review

e575f5d

Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

update default demo parameters

007706c

scripts

1167199

alexfauquette merged commit 5196d57 into mui:next Dec 8, 2023
17 checks passed

alexfauquette deleted the scatter-interaction branch December 8, 2023 09:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[charts] Use voronoi cells to trigger interaction with scatter items #10981

[charts] Use voronoi cells to trigger interaction with scatter items #10981

alexfauquette commented Nov 9, 2023 •

edited by LukasTy

mui-bot commented Nov 9, 2023 •

edited

github-actions bot commented Nov 20, 2023

alexfauquette Dec 4, 2023

LukasTy Dec 7, 2023

alexfauquette Dec 7, 2023

LukasTy Dec 7, 2023

alexfauquette Dec 4, 2023

alexfauquette Dec 4, 2023

LukasTy left a comment

LukasTy Dec 7, 2023

alexfauquette Dec 7, 2023

LukasTy Dec 7, 2023

LukasTy Dec 7, 2023

alexfauquette Dec 7, 2023

LukasTy Dec 7, 2023

LukasTy Dec 7, 2023

	r={(dataPoint.isHighlighted ? 1.2 : 1) * markerSize}
	r={markerSize}

[charts] Use voronoi cells to trigger interaction with scatter items #10981

[charts] Use voronoi cells to trigger interaction with scatter items #10981

Conversation

alexfauquette commented Nov 9, 2023 • edited by LukasTy

Changelog

mui-bot commented Nov 9, 2023 • edited

github-actions bot commented Nov 20, 2023

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

LukasTy 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

alexfauquette commented Nov 9, 2023 •

edited by LukasTy

mui-bot commented Nov 9, 2023 •

edited