[charts] Improve zoomed highlight behaviour #13868

JCQuintas · 2024-07-17T11:13:33Z

BarChart
- highlight: Move the highlight component inside the clip group
- tooltip: No change needed, as when the bars are fully outside the tooltip doesn't trigger.
LineChart
- highlight: Move the highlight component inside the clip group
- tooltip: Left as is. When you are zoomed in you might still want to see the value of what is outside
ScatterChart
- highlight: none
- tooltip: Changed the voronoi/delauney to only consider the points inside render area

JCQuintas · 2024-07-17T11:17:27Z

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

+        const pointY = getYPosition(y);
+
+        if (pointX < left || pointX > left + width || pointY < top || pointY > top + height) {
+          return [0, 0];


Not sure this is the best approach. I've tried:

Filtering: Messes up index value logic

Retuning [NaN, NaN]: Seem to go into an infinite loop

Retuning [(+-)Infinity, (+-)Infinity]: In any combination it breaks because the Delaunator logic uses infinity itself to check initial values, so whenever we zoom, we are always on the min or max of the values

Returning [0,0]: Seems to work correctly and not affect anything else. I've tried with multiple points combination

You move all the outsider in a trash point (0, 0)

This work as long as there is a point in the drawing area which is closer to the pointer than the top left corner of the SVG.

Basically if you have not point in the drawing area and the red circle, you will start triggering random point.

I see few options:

A proper filter with a mapping from "index after filtering" to "index before filtering"

You move point to (-width, -height) instead of (0, 0) The only way for them to be trigger is that all points are outside of the drawing area

You don't filter, and if the point is outside you use the neighbors method to implement A* algo and find the closes inside the drawing are (probably too complex)

🙃 I was thing [0,0] in dataspace, not svgspace. Feels dumb now. lol

I feel option 2 will be more reasonable, since we won't have to maintain two filters... One of the issue is that Delauney.find() will return an index, if we can't map it between the two of them easily, there wont be a lot of value in that.

mui-bot · 2024-07-17T11:17:36Z

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

Generated by 🚫 dangerJS against e5306c8

JCQuintas · 2024-07-17T11:18:26Z

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

        return 'outside-chart';
      }

      if (!delauneyRef.current) {
        return 'no-point-found';
      }

-      const closestPointIndex = delauneyRef.current.find(svgPoint.x, svgPoint.y);
+      const closestPointIndex = delauneyRef.current.find(svgPoint.x, svgPoint.y, lastFind.current);


Started storing/reusing last index and removed the TODO

JCQuintas · 2024-07-17T11:19:07Z

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

@@ -180,11 +192,11 @@ function ChartsVoronoiHandler(props: ChartsVoronoiHandlerProps) {
      onItemClick(event, { type: 'scatter', seriesId, dataIndex });
    };

-    element.addEventListener('pointerout', handleMouseOut);
+    element.addEventListener('pointerleave', handleMouseLeave);


out>leave as out triggers on every element inside the chart, rather than leaving the main svg

alexfauquette · 2024-07-17T12:45:35Z

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

+        <LineHighlightPlot {...lineHighlightPlotProps} />
      </g>
      <ChartsAxis {...chartsAxisProps} />
-      <ChartsAxisHighlight {...axisHighlightProps} />
      <MarkPlot {...markPlotProps} />


LineHighlightPlot needs to be on top of the MarkPlot. Otherwise, the mark plot will hidde it.
For example

It should get similar filtering isInRange as the MarkPlot

https://github.com/mui/mui-x/blob/master/packages/x-charts/src/LineChart/MarkPlot.tsx/#L93-L103

Moved the point inside logic to a new function inside the DrawingContext because it was reused in multiple places

alexfauquette · 2024-07-17T13:46:53Z

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

+        const pointY = getYPosition(y);
+
+        if (pointX < left || pointX > left + width || pointY < top || pointY > top + height) {
+          return [0, 0];


You move all the outsider in a trash point (0, 0)

This work as long as there is a point in the drawing area which is closer to the pointer than the top left corner of the SVG.

Basically if you have not point in the drawing area and the red circle, you will start triggering random point.

I see few options:

A proper filter with a mapping from "index after filtering" to "index before filtering"

You move point to (-width, -height) instead of (0, 0) The only way for them to be trigger is that all points are outside of the drawing area

You don't filter, and if the point is outside you use the neighbors method to implement A* algo and find the closes inside the drawing are (probably too complex)

This reverts commit a066ec4.

alexfauquette

Nice fix 👌

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

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

…r.tsx Co-authored-by: Alexandre Fauquette <[email protected]> Signed-off-by: Jose C Quintas Jr <[email protected]>

Signed-off-by: Jose C Quintas Jr <[email protected]> Co-authored-by: Alexandre Fauquette <[email protected]>

JCQuintas added 5 commits July 17, 2024 13:06

Put bar axis highlight inside the clip group

8af9065

Put line axis highlight inside the clip group

a066ec4

Disable voronoi outside render area

4a5ebb2

Use last found point as start of next search

24479d5

Disable voronoi points outside render view

35df640

JCQuintas added enhancement This is not a bug, nor a new feature component: charts This is the name of the generic UI component, not the React module! labels Jul 17, 2024

JCQuintas self-assigned this Jul 17, 2024

JCQuintas commented Jul 17, 2024

View reviewed changes

JCQuintas requested a review from alexfauquette July 17, 2024 11:19

alexfauquette reviewed Jul 17, 2024

View reviewed changes

JCQuintas added 7 commits July 17, 2024 16:02

Revert "Put line axis highlight inside the clip group"

569c87a

This reverts commit a066ec4.

Create isInside helper in drawing provider

455b502

Use new isPointInside for line highlight

5c1379c

Fix voronoi

1362689

use isPointInside in other places

818316d

Use isPointInside on zoom hooks

88b4a6c

reset lastFind when re-calculating

eda415d

JCQuintas requested a review from alexfauquette July 17, 2024 16:00

Fix issues

d2e87ba

alexfauquette approved these changes Jul 18, 2024

View reviewed changes

packages/x-charts-pro/src/BarChartPro/BarChartPro.tsx Show resolved Hide resolved

packages/x-charts/src/ChartsVoronoiHandler/ChartsVoronoiHandler.tsx Outdated Show resolved Hide resolved

Update packages/x-charts/src/ChartsVoronoiHandler/ChartsVoronoiHandle…

e5306c8

…r.tsx Co-authored-by: Alexandre Fauquette <[email protected]> Signed-off-by: Jose C Quintas Jr <[email protected]>

JCQuintas enabled auto-merge (squash) July 18, 2024 08:56

JCQuintas merged commit cd68580 into mui:master Jul 18, 2024
15 checks passed

JCQuintas deleted the fix-zoom-highlights branch July 18, 2024 09:13

DungTiger pushed a commit to DungTiger/mui-x that referenced this pull request Jul 23, 2024

[charts] Improve zoomed highlight behaviour (mui#13868)

227e263

Signed-off-by: Jose C Quintas Jr <[email protected]> Co-authored-by: Alexandre Fauquette <[email protected]>

alhabib5565 mentioned this pull request Aug 19, 2024

[Snyk] Upgrade @mui/x-data-grid from 7.3.0 to 7.11.1 alhabib5565/quick-shop-client#11

Open

thomasmoon pushed a commit to thomasmoon/mui-x that referenced this pull request Sep 9, 2024

[charts] Improve zoomed highlight behaviour (mui#13868)

13543a3

Signed-off-by: Jose C Quintas Jr <[email protected]> Co-authored-by: Alexandre Fauquette <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[charts] Improve zoomed highlight behaviour #13868

[charts] Improve zoomed highlight behaviour #13868

JCQuintas commented Jul 17, 2024

JCQuintas Jul 17, 2024 •

edited

Loading

alexfauquette Jul 17, 2024

JCQuintas Jul 17, 2024

mui-bot commented Jul 17, 2024 •

edited

Loading

JCQuintas Jul 17, 2024

JCQuintas Jul 17, 2024

alexfauquette Jul 17, 2024

JCQuintas Jul 17, 2024

alexfauquette Jul 17, 2024

alexfauquette left a comment

[charts] Improve zoomed highlight behaviour #13868

[charts] Improve zoomed highlight behaviour #13868

Conversation

JCQuintas commented Jul 17, 2024

JCQuintas Jul 17, 2024 • edited Loading

Choose a reason for hiding this comment

alexfauquette Jul 17, 2024

Choose a reason for hiding this comment

JCQuintas Jul 17, 2024

Choose a reason for hiding this comment

mui-bot commented Jul 17, 2024 • edited Loading

JCQuintas Jul 17, 2024

Choose a reason for hiding this comment

JCQuintas Jul 17, 2024

Choose a reason for hiding this comment

alexfauquette Jul 17, 2024

Choose a reason for hiding this comment

JCQuintas Jul 17, 2024

Choose a reason for hiding this comment

alexfauquette Jul 17, 2024

Choose a reason for hiding this comment

alexfauquette left a comment

Choose a reason for hiding this comment

JCQuintas Jul 17, 2024 •

edited

Loading

mui-bot commented Jul 17, 2024 •

edited

Loading