mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.79k stars 1.12k forks source link

[charts] LinePlot onItemClick should provide dataIndex property #12840

Open dmytro-shchurov opened 2 weeks ago

dmytro-shchurov commented 2 weeks ago

Summary

When the composition is used, then LinePlot onItemClick callback does not have dataIndex property provided. At the same time ChartContainer does not receive any events, so it's impossible to handle onAxisClick event as well.

Examples

<ResponsiveChartContainer 
  ...
  onAreaClick // is never triggered
  onLineClick // is never triggered
  onMarkClick // is never triggered
  onAxisClick // is never triggered

>
  <LinePlot onItemClick={(_, { seriesId, dataIndex <- is always undefined })} .. />
  ...
</ResponsiveChartContainer>

Motivation

For relatively small number of points, to have MarkPlot child component is to have some disadvantages like a significant drop in a performance (for already 150 marks which is not a really huge number), and mark circles everywhere which are not configurable.

Together with missing a click event handler for ChartsXAxis component, and a silent onAxisClick and onItemClick in a container, a silent LineHighlightPlot, ChartsAxisHighlight and ChartsTooltip which could theoretically give a chance to keep a highlighted data point in a state, that causes big problems in determine a data point under a cursor when is clicked.

Consider pls a possibility to return the dataIndex property from LinePlot onItemClick event in case user clicks not on a line itself but on a data point (with some tolerance). I understand that LinePlot responsibility is only about a connecting line, but what can I do if there are NO other ways to get a data point index?..

As an option pls consider firing onItemClick event in ChartContainer even when a composition is used.

Search keywords: charts, onItemClick, ChartContainer, LinePlot

JCQuintas commented 2 weeks ago

Hi @dmytro-shchurov, right now we don't support onAreaClick|onLineClick|onMarkClick|onAxisClick on the ResponsiveChartContainer.

And dataIndex can be undefined when hovering an area not directly above a point.

Please let me know if this information helps you out. If not, would it be possible to provide a sample of your issue on https://codesandbox.io/ or https://stackblitz.com/ ? This way we can better understand your problem and help you out. 😄

JCQuintas commented 2 weeks ago

@alexfauquette we probably need to export the LineItemIdentifier to the docs at https://mui.com/x/api/charts/line-plot/

dmytro-shchurov commented 2 weeks ago

At the moment I use this workaround (just in case someone will be interested)

const highlightedIndex = useRef(null);

<ResponsiveChartContainer
 ...
  <LineHighlightPlot
              slots={{
                lineHighlight: ({ x, y, id }) => 
                    <CustomColoredDot id={id} x={x} y={y} dataIndex={highlightedIndex.current} onClick={handlePointClick}/>
              }}
            />
  <ChartsTooltip
              trigger="axis"
              slots={{
                axisContent: ({ dataIndex, ...props }) => {
                  highlightedIndex.current = dataIndex;
                  return (
                    <DefaultChartsAxisTooltipContent
                      dataIndex={dataIndex}
                      {...props}
                    />
                  );
                },
              }}
            />
alexfauquette commented 2 weeks ago
<ResponsiveChartContainer 
  ...
  onAreaClick // is never triggered
  onLineClick // is never triggered
  onMarkClick // is never triggered
  onAxisClick // is never triggered

>
  <LinePlot onItemClick={(_, { seriesId, dataIndex <- is always undefined })} .. />
  ...
</ResponsiveChartContainer>

I don't see the interest in passing those props to the ResponsiveChartContainer. Those event are only about <LinePlot /> If you look at the implementation of <LineChart />, we just propagate the onLineClick to the LInePlot.onItemClick

https://github.com/mui/mui-x/blob/f3b5f1b40ecb0a853f1cef841db533e3aec33c55/packages/x-charts/src/LineChart/LineChart.tsx#L205-L210

or already 150 marks which is not a realy huge number

A chart with 150 dots seems to be a lot. the showMark could help you to reduce this amount (it does not remove the highlight)

Together with missing a click event handler for ChartsXAxis component, and a silent onAxisClick and onItemClick in a container, a silent LineHighlightPlot, ChartsAxisHighlight and ChartsTooltip which could theoretically give a chance to keep a highlighted data point in a state, that causes big problems in determine a data point under a cursor when is clicked.

When using composition those events call back are not to put in the container. They have dedicated components such that the user who don't need is not forced to have in his bundle

<ChartsOnAxisClickHandler onAxisClick={onAxisClick} />

Consider pls a possibility to return the dataIndex property from LinePlot onItemClick event in case user clicks not on a line itself but on a data point (with some tolerance).

Adding tolerences on the line is tricky because we listen the the onClick event from the <path /> component. Which does not allow this notion of "tolerance"

dmytro-shchurov commented 2 weeks ago

A chart with 150 dots seems to be a lot. the showMark (https://mui.com/x/react-charts/lines/#optimization) could help you to reduce this amount That would help, and I've already tried this, but a sad thing is that when there is no mark element, nothing can handle onClick event. In my case a performance is acceptable when I display every second mark, but unfortunatelly some of missed marks are at extreme values, which user is the most interested to click, and find out more info about the extreme point. I also tried to use MarkPlot slot attribute to display only a single mark (to mimic a line hightlight element). But this causes a re-render on mouse move (and again a mark element is not enough customizable) with some race collision errors in a console. My workaround above is more or less fine. But 1) it depends on the tooltip and will not work on mobile devices 2) it looks kind of ugly May be it makes a sense to add a data point info and a click calback to LineHighlightPlot? I'm afraid you need to handle a mouse click on a surface, calculate a data point underneeth, and create an item click event in a context, which multiple chart objects can process then. Or just add a callback to a series object. You've separated a line plot, a mark plot, and a highlight plot, and this causes these problems. Rechart, which has the coolest API in the world, has only a line object with a customisable active/inactive dot/mark. Unfortunatelly it's not MUI-ready. On Apr 19 2024, at 2:27 pm, Alexandre Fauquette @.***> wrote:

<ResponsiveChartContainer ... onAreaClick // is never triggered onLineClick // is never triggered onMarkClick // is never triggered onAxisClick // is never triggered

<LinePlot onItemClick={(_, { seriesId, dataIndex <- is always undefined })} .. /> ... I don't see the interest in passing those props to the ResponsiveChartContainer. Those event are only about If you look at the implementation of , we just propagate the onLineClick to the LInePlot.onItemClick

https://github.com/mui/mui-x/blob/f3b5f1b40ecb0a853f1cef841db533e3aec33c55/packages/x-charts/src/LineChart/LineChart.tsx#L205-L210

or already 150 marks which is not a realy huge number A chart with 150 dots seems to be a lot. the showMark (https://mui.com/x/react-charts/lines/#optimization) could help you to reduce this amount (it does not remove the highlight)

Together with missing a click event handler for ChartsXAxis component, and a silent onAxisClick and onItemClick in a container, a silent LineHighlightPlot, ChartsAxisHighlight and ChartsTooltip which could theoretically give a chance to keep a highlighted data point in a state, that causes big problems in determine a data point under a cursor when is clicked. When using composition (https://mui.com/x/react-charts/lines/#composition) those events call back are not to put in the container. They have dedicated components such that the user who don't need is not forced to have in his bundle

Consider pls a possibility to return the dataIndex property from LinePlot onItemClick event in case user clicks not on a line itself but on a data point (with some tolerance). Adding tolerences on the line is tricky because we listen the the onClick event from the component. Which does not allow this notion of "tolerance"

— Reply to this email directly, view it on GitHub (https://github.com/mui/mui-x/issues/12840#issuecomment-2066465234), or unsubscribe (https://github.com/notifications/unsubscribe-auth/AAF4RRP4X2EJH2LACBSJVF3Y6EEU7AVCNFSM6AAAAABGOUFTXKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRWGQ3DKMRTGQ). You are receiving this because you were mentioned.