Skip to content

[charts][docs] Document click listener for scatter composition - #22809

Merged
sai6855 merged 7 commits into
mui:masterfrom
sai6855:scatter-click
Jun 16, 2026
Merged

[charts][docs] Document click listener for scatter composition#22809
sai6855 merged 7 commits into
mui:masterfrom
sai6855:scatter-click

Conversation

@sai6855

@sai6855 sai6855 commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

closes #11933

In addition to this PR change, should we also disallow passing onItemClick on ChartsDataProvider when disableHitArea is true through typescript?

preview: https://deploy-preview-22809--material-ui-x.netlify.app/x/react-charts/scatter/#handling-clicks

Preview: https://deploy-preview-22809--material-ui-x.netlify.app/x/react-charts/scatter/

@sai6855
sai6855 requested a review from Copilot June 15, 2026 07:40
@sai6855 sai6855 added docs Improvements or additions to the documentation. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. scope: charts Changes related to the charts. labels Jun 15, 2026
Copilot AI reviewed Jun 15, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates the Scatter chart documentation to clarify how onItemClick is wired when using the Scatter chart in a composed setup (e.g. ChartsDataProvider + ScatterPlot), addressing the need to document the special click-listener behavior.

Changes:

  • Adds a cross-reference from the “Click event” section to composition-specific click handling guidance.
  • Introduces a new “Handling clicks” subsection under “Composition” describing where to attach onItemClick depending on hit-area behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/data/charts/scatter/scatter.md Outdated
<ChartsDataProvider onItemClick={handleItemClick}>{/* ... */}</ChartsDataProvider>
```

When `disableHitArea` is `true` (with the default `svg-single` renderer), the user must click directly on a marker, so pass `onItemClick` to `ScatterPlot` instead.
@code-infra-dashboard

code-infra-dashboard Bot commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Deploy preview

Bundle size

Bundle Parsed size Gzip size
@mui/x-data-grid 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-pro 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-premium 0B(0.00%) 0B(0.00%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-charts-premium 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)
@mui/x-license 0B(0.00%) 0B(0.00%)

Details of bundle changes

Performance

Total duration: 2,249.82 ms +244.31 ms(+12.2%) | Renders: 67 (+0)

Test Duration Renders
Heatmap: 100x100 grid 828.13 ms 🔺+237.99 ms(+40.3%) 2 (+0)
SparkLineChart grid of 100 instances 214.45 ms 🔺+68.74 ms(+47.2%) 1 (+0)
RangeBarChart with big data amount 119.06 ms 🔺+48.44 ms(+68.6%) 3 (+0)
BarChartPro with big data amount 62.34 ms 🔺+27.71 ms(+80.0%) 3 (+0)
HeatmapPremium: 100x100 grid (webgl renderer) 56.67 ms 🔺+18.46 ms(+48.3%) 5 (+0)

…and 8 more (+13 within noise) — details

Metric alarms

Test Metric Change
Heatmap: 100x100 grid bench:paint 🔺 +280.90 ms
SparkLineChart grid of 100 instances bench:paint 🔺 +250.25 ms
RangeBarChart with big data amount bench:paint 🔺 +79.81 ms
BarChartPro with big data amount bench:paint 🔺 +51.78 ms
HeatmapPremium: 100x100 grid (webgl renderer) bench:paint 🔺 +21.30 ms
RadialBarChart stacked with multiple series bench:paint 🔺 +20.23 ms
LineChartPro with big data amount and zoomed in (with marks) bench:paint 🔺 +14.88 ms
SankeyChart with big data amount bench:paint 🔺 +8.02 ms

Check out the code infra dashboard for more information about this PR.

@sai6855
sai6855 marked this pull request as ready for review June 15, 2026 07:58
@JCQuintas

JCQuintas commented Jun 15, 2026

Copy link
Copy Markdown
Member

@alexfauquette I suppose we will leverage the "calculate position" method more often (webgl, new chart types, etc), should we look at reworking the api instead?

Else we will be in this state where "if this, then that, if that, then this"

@alexfauquette

alexfauquette commented Jun 15, 2026

Copy link
Copy Markdown
Member

should we look at reworking the api instead?

Looks like we can keep the API as it is. and do not document the ScatterPlot onItemClick except in its API page

We could remove it in the v10 for API simplification

@sai6855

sai6855 commented Jun 15, 2026

Copy link
Copy Markdown
Member Author

should we look at reworking the api instead?

Looks like we can keep the API as it is. and do not document the ScatterPlot onItemClick except in its API page

We could remove it in the v10 for API simplification

Added here https://deploy-preview-22809--material-ui-x.netlify.app/x/api/charts/scatter-plot/#scatter-plot-prop-onItemClick

alexfauquette
alexfauquette reviewed Jun 16, 2026
View reviewed changes
Comment thread packages/x-charts/src/ScatterChart/ScatterPlot.tsx
Comment on lines +38 to +42
/**
* Callback fired when a marker is clicked directly and `disableHitArea` is `true`.
* When `disableHitArea` is `false`, use `onItemClick` on the chart container to handle closest-point clicks.
* @param {MouseEvent} event Mouse event recorded on the `<svg/>` element.
* @param {ScatterItemIdentifier} scatterItemIdentifier The scatter item identifier.

@alexfauquette alexfauquette Jun 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true but at the ScatterChart level.

For the props of the ScatterPlot, the onItemClick will be trigger independently from the disableHitArea value.

This logic is just in the useScatterChartProps that

  • if disableHitArea=true => the onItemClick is propagated to the ScatterPlot
  • if disableHitArea=false => the onItemClick is propagated to the DataProvider

https://github.com/mui/mui-x/blob/master/packages/x-charts/src/ScatterChart/useScatterChartProps.ts#L75-L77

@sai6855 sai6855 Jun 16, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt now?

image

alexfauquette
alexfauquette approved these changes Jun 16, 2026
View reviewed changes

@alexfauquette alexfauquette left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While reviewing I noticed we already have a docs section about it 🫣
https://mui.com/x/react-charts/scatter/#click-event

So I just added some additoinal option to play with it

@sai6855
sai6855 merged commit 34bbeb1 into mui:master Jun 16, 2026
21 checks passed
mbrookes pushed a commit to mbrookes/mui-x that referenced this pull request Jun 27, 2026
@sai6855 @alexfauquette
[charts][docs] Document click listener for scatter composition (mui#2…
eb53eb8 
…2809)

Co-authored-by: alex <alex.fauquette@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@alexfauquette alexfauquette alexfauquette approved these changes
@JCQuintas JCQuintas Awaiting requested review from JCQuintas JCQuintas is a code owner

Assignees

No one assigned

Labels

docs Improvements or additions to the documentation. scope: charts Changes related to the charts. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[charts][docs] Document click listener for scatter composition

4 participants

@sai6855 @JCQuintas @alexfauquette