[charts] Add configurable slots to toolbar #17712

bernardobelchior · 2025-05-06T09:12:14Z

Part of #17557.

Add toolbar slots to all charts that render a toolbar.

This PR introduces a different way to define slots, which I'm not sure about. I'd like to hear your thoughts in this comment.

This follow-up PR contains an example on how to use the different methods: slots and render props.

mui-bot · 2025-05-06T09:15:43Z

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

Bundle size report

Total Size Change:^{${\tiny{\color{red}▲}}$}+12.4KB^(+0.10%) - Total Gzip Change:^{${\tiny{\color{red}▲}}$}+4.1KB^(+0.11%)
Files: 118 total (0 added, 0 removed, 17 changed)

@mui/x-charts-pro/ChartContainerPro parsed:^{${\tiny{\color{red}▲}}$}+1.27KB^(+0.90%) gzip:^{${\tiny{\color{red}▲}}$}+461B^(+0.97%)
@mui/x-charts-pro/FunnelChart parsed:^{${\tiny{\color{red}▲}}$}+1.27KB^(+0.63%) gzip:^{${\tiny{\color{red}▲}}$}+488B^(+0.75%)
@mui/x-charts-pro/Heatmap parsed:^{${\tiny{\color{red}▲}}$}+1.21KB^(+0.66%) gzip:^{${\tiny{\color{red}▲}}$}+451B^(+0.75%)
@mui/x-charts-pro/ChartDataProviderPro parsed:^{${\tiny{\color{red}▲}}$}+1.2KB^(+0.90%) gzip:^{${\tiny{\color{red}▲}}$}+439B^(+0.98%)
@mui/x-charts-pro parsed:^{${\tiny{\color{red}▲}}$}+1.01KB^(+0.31%) gzip:^{${\tiny{\color{red}▲}}$}+293B^(+0.30%)
@mui/x-charts parsed:^{${\tiny{\color{red}▲}}$}+830B^(+0.32%) gzip:^{${\tiny{\color{red}▲}}$}+219B^(+0.27%)
@mui/x-charts/ScatterChart parsed:^{${\tiny{\color{red}▲}}$}+600B^(+0.38%) gzip:^{${\tiny{\color{red}▲}}$}+179B^(+0.35%)
@mui/x-charts/PieChart parsed:^{${\tiny{\color{red}▲}}$}+582B^(+0.36%) gzip:^{${\tiny{\color{red}▲}}$}+199B^(+0.38%)
@mui/x-charts/LineChart parsed:^{${\tiny{\color{red}▲}}$}+581B^(+0.31%) gzip:^{${\tiny{\color{red}▲}}$}+148B^(+0.24%)
@mui/x-charts/BarChart parsed:^{${\tiny{\color{red}▲}}$}+580B^(+0.34%) gzip:^{${\tiny{\color{red}▲}}$}+169B^(+0.30%)

Show 7 more bundle changes

@mui/x-charts/RadarChart parsed:^{${\tiny{\color{red}▲}}$}+520B^(+0.33%) gzip:^{${\tiny{\color{red}▲}}$}+196B^(+0.38%)
@mui/x-charts-pro/ScatterChartPro parsed:^{${\tiny{\color{red}▲}}$}+512B^(+0.27%) gzip:^{${\tiny{\color{red}▲}}$}+136B^(+0.21%)
@mui/x-charts-pro/BarChartPro parsed:^{${\tiny{\color{red}▲}}$}+492B^(+0.24%) gzip:^{${\tiny{\color{red}▲}}$}+140B^(+0.21%)
@mui/x-charts-pro/LineChartPro parsed:^{${\tiny{\color{red}▲}}$}+492B^(+0.22%) gzip:^{${\tiny{\color{red}▲}}$}+136B^(+0.19%)
@mui/x-charts/ChartContainer parsed:^{${\tiny{\color{red}▲}}$}+457B^(+0.41%) gzip:^{${\tiny{\color{red}▲}}$}+149B^(+0.39%)
@mui/x-charts/SparkLineChart parsed:^{${\tiny{\color{red}▲}}$}+419B^(+0.24%) gzip:^{${\tiny{\color{red}▲}}$}+144B^(+0.25%)
@mui/x-charts/ChartDataProvider parsed:^{${\tiny{\color{red}▲}}$}+417B^(+0.40%) gzip:^{${\tiny{\color{red}▲}}$}+151B^(+0.42%)

Details of bundle changes

Generated by 🚫 dangerJS against c25776e

codspeed-hq · 2025-05-06T09:20:03Z

CodSpeed Performance Report

Merging #17712 will not alter performance

_{Comparing bernardobelchior:charts-slot-context (c25776e) with master (48666ad)}

Summary

✅ 9 untouched benchmarks

github-actions · 2025-05-06T09:50:30Z

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

bernardobelchior · 2025-05-06T11:42:16Z

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

+    /**
+     * Slots to customize charts' components.
+     */
+    slots?: Partial<ChartsSlots>;
+    /**
+     * The props for the slots.
+     */
+    slotProps?: Partial<ChartsSlotProps>;
+  };


This is a break away from the current pattern of only providing slots to the higher-level chart components (e.g., LineChart) or providing slots directly to the components that render them (e.g., ChartsXAxis).

I'd like your opinion on this.

The reason I chose this option is because it allows defining the base slots (e.g., icons, buttons, tooltips) for the whole chart. This means that all toolbar components (and potentially others in the future) will use these components.

This contrasts with the current approach, which would be to pass the slots directly to the toolbar and, if using composition, to its descendants as well.

My reasoning was: if we're going to override the components using slots, might as well do it for all components in a chart. If we want to override one by one, we can use composition and the render prop if needed.

If we kept the current approach, overriding the slots when using composition would be cumbersome because we would need to set the slots in all components:

function CustomToolbar() { return <Toolbar> <ChartToolbarZoomInButton slots={{ baseIconButton: CustomButton }} /> <ChartToolbarZoomOutButton slots={{ baseIconButton: CustomButton }} /> </Toolbar> }

With the approach taken in this PR, there's a single place to define the slots for base components, such as icons, buttons and tooltips.

I understand this will create two ways of using slots:

The current one for all components;

The new one for the toolbar.

And that's where I'd like to hear your opinion. Do you think it's better to strive for consistency in exchange for convenience? What are your thoughts?

For context, here's a PR that implements this pattern and the render prop, along with a demo to see how it looks like.

I'll have to sleep on the idea 😆

It seems an interesting approach, but it might be annoying to have both at the same time, as you mentioned 🤔

@JCQuintas have you slept on the idea? What did you dream about? 😛

I like the idea of moving it to a single place, but I'm not sure it would be the best moment to do so.

I think the current idea is to still, eventually, provide a more base-ui like offering for charts, this would mean that the slots go away in favour of composition, at least in that version.

I'm not sure if having a global context for slots would make this easier or harder tbh 🤔

Yeah, I don't like the idea of having a different pattern either.

I think the current idea is to still, eventually, provide a more base-ui like offering for charts, this would mean that the slots go away in favour of composition, at least in that version.

How so? The idea of this approach is to be similar to the Data Grid approach, which is supposed to be design system agnostic.

So, concluding, you think it's better to keep the current pattern instead of introducing a new one, is that it?

I would like to hear what @alexfauquette has to say, as he might be more informed on this

I don't see issues with having both strategies since they solve two distinct use-cases:

Direct slot propagation for big component: You want to replace the tooltip, legend, overlay. We can make it easy since their is only one of them in the chart.

You want to replace a MUI component: They can be multiple so the root props are propagated thanks to a context to be transparent.

This was further discussed in this PR.

alexfauquette

I'm good with the idea

alexfauquette · 2025-05-14T08:51:58Z

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

+    /**
+     * Slots to customize charts' components.
+     */
+    slots?: Partial<ChartsSlots>;
+    /**
+     * The props for the slots.
+     */
+    slotProps?: Partial<ChartsSlotProps>;
+  };


I don't see issues with having both strategies since they solve two distinct use-cases:

Direct slot propagation for big component: You want to replace the tooltip, legend, overlay. We can make it easy since their is only one of them in the chart.

You want to replace a MUI component: They can be multiple so the root props are propagated thanks to a context to be transparent.

packages/x-charts-pro/src/FunnelChart/funnelSlots.types.ts

alexfauquette · 2025-05-14T09:33:14Z

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

+
+/**
+ * Get the slots and slotProps from the nearest `ChartDataProvider` or `ChartDataProviderPro`.
+ * @returns {ChartsSlotsContextValue} The slots and slotProps from the context.
+ */
+export function useChartToolbarSlots<


Not sure it should be associated with the Toolbar.

This could be extended to all MUI components such that we can also remove the Paper, Popper, Typography.

Not sure also it should be exported, because I don't see a usage we could encourage.
The slots are here to let user the ability to override our default components.

For example slots={{ iconButton: MyCompanyButton }}

If they are at the level of creating their own component, they will either:

Reuse our component, but then it's transparent <ChartsDownloadButton /> The rendered button already come from the context

Reuse our component with render <ChartsDownloadButton render={() => <MyCompanyButton .../>}/>

Do their own stuff MyToolbarButton = () => Button<MyCompanyButton .../>

The only use cases if they want to have built-in way to switch from one set of components to another

Not sure it should be associated with the Toolbar.

Yeah, I had that doubt as well. We'll probably want to use this for more things, so I think it's better to be generic.

This could be extended to all MUI components such that we can also remove the Paper, Popper, Typography.

Good point 👍 we'll probably want to do that.

Not sure also it should be exported, because I don't see a usage we could encourage.

If we're able to set them in the ChartsDataProvider, I agree there's no point in exporting the ChartsSlotsContext. Otherwise, I think we'll need to export it so that users can replace Material components with their own when using composition.

github-actions · 2025-05-20T13:56:08Z

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

github-actions · 2025-05-22T13:06:02Z

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

alexfauquette

Looks nice, I'm impatient to see how this DX will evolve 👍

It might need some extra love in the documentation to avoid confusion

alexfauquette · 2025-05-23T07:18:40Z

packages/x-charts-pro/src/FunnelChart/funnelSlots.types.ts

+    ChartsAxisSlots,
+    ChartsToolbarSlots,
+    Partial<ChartsSlots> {}


Is it to anticipate the addition of chart export for the Funnel in an upcoming PR?

Because here you add slots that have no effect on the chart

Which slots have no effect on the funnel?

These are the toolbar's slots, can't all charts accept a toolbar, in theory?

bernardobelchior · 2025-05-26T07:33:12Z

Looks nice, I'm impatient to see how this DX will evolve 👍

Thanks! Yeah, I think it'll be nice!

It might need some extra love in the documentation to avoid confusion

Yeah, I'm adding some more docs in this PR. Do you think there's still some parts that should be better documented?

bernardobelchior added type: new feature Expand the scope of the product to solve a new problem. scope: charts Changes related to the charts. labels May 6, 2025

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

bernardobelchior force-pushed the charts-slot-context branch from 30611b5 to a80b544 Compare May 6, 2025 09:57

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

bernardobelchior commented May 6, 2025

View reviewed changes

bernardobelchior requested a review from JCQuintas May 9, 2025 10:58

alexfauquette reviewed May 14, 2025

View reviewed changes

bernardobelchior force-pushed the charts-slot-context branch from 6187353 to 298bf52 Compare May 19, 2025 09:17

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

bernardobelchior force-pushed the charts-slot-context branch from 3a69d4f to 2df9493 Compare May 21, 2025 07:52

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

bernardobelchior marked this pull request as ready for review May 21, 2025 08:23

bernardobelchior requested a review from alexfauquette May 21, 2025 08:23

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

[charts] Add configurable slots to toolbar

c25776e

bernardobelchior force-pushed the charts-slot-context branch from 2df9493 to c25776e Compare May 22, 2025 14:07

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

alexfauquette approved these changes May 23, 2025

View reviewed changes

bernardobelchior merged commit ea53ddc into mui:master May 26, 2025
24 checks passed

bernardobelchior deleted the charts-slot-context branch May 26, 2025 07:33

Uh oh!

[charts] Add configurable slots to toolbar #17712

[charts] Add configurable slots to toolbar #17712

Uh oh!

Conversation

bernardobelchior commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mui-bot commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle size report

Uh oh!

codspeed-hq bot commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #17712 will not alter performance

Summary

Uh oh!

github-actions bot commented May 6, 2025

Uh oh!

bernardobelchior May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexfauquette left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 20, 2025

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

alexfauquette left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bernardobelchior commented May 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bernardobelchior commented May 6, 2025 •

edited

Loading

mui-bot commented May 6, 2025 •

edited

Loading

codspeed-hq bot commented May 6, 2025 •

edited

Loading

bernardobelchior May 6, 2025 •

edited

Loading