Skip to content

[charts] Create charts toolbar #17557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bernardobelchior opened this issue Apr 25, 2025 · 6 comments
Open

[charts] Create charts toolbar #17557

bernardobelchior opened this issue Apr 25, 2025 · 6 comments
Assignees
Labels
component: charts This is the name of the generic UI component, not the React module! new feature New feature or request

Comments

@bernardobelchior
Copy link
Member

bernardobelchior commented Apr 25, 2025

Similar to the data grid's toolbar.

Image

We probably want to hide the toolbar when exporting.

Unblocks:

@github-actions github-actions bot added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 25, 2025
@bernardobelchior bernardobelchior self-assigned this Apr 25, 2025
@zannager zannager added the component: charts This is the name of the generic UI component, not the React module! label Apr 25, 2025
@bernardobelchior
Copy link
Member Author

The Data Grid's toolbar follows a composable approach that relies on slots and the render prop. The render prop uses the useGridComponentRenderer hook to render components (source).

Here's an example of how to use custom components in the Data Grid's toolbar.

To me, it seems like the container/presentational component split: the Data-Grid-exported components are containers (e.g., QuickFilter, QuickFilterTrigger), while the components received as render props are presentational components.

The Data Grid uses a GridToolbar internally, but that can be replaced with a custom version using the toolbar slot. For components that are used in GridToolbar, but aren't required for the toolbar to work (e.g., tooltips), the Data Grid uses slots. For components that are required in both the GridToolbar and in the Toolbar components, a component is exported that supports the render prop, e.g., ToolbarButton, FilterPanelTrigger.

The Toolbar component also comes with accessible buttons out of the box, even for custom components. This allows accessibility to be automatically implemented without needing user's intervention.

For slots that are used in the GridToolbar, the Data Grid defines an API that seems to be a copy of Material UI's props. These can be overridden by the user, and default to the MUI version if no override is made.

Concluding, while complex, the Data Grid toolbar seems to be composable, customizable and include accessibility out of the box.

Unfortunately, since the fallback components can be rendered at runtime (i.e., bundlers can't know whether they're used or not), I think bundlers won't be able to tree-shake @mui/material even if users override all slots that use Material UI.

@alexfauquette
Copy link
Member

Concluding, while complex, the Data Grid toolbar seems to be composable, customizable and include accessibility out of the box.

If we look from a usecase perspective, you have:

  1. Using the same behavior and layout but different design with rootProps.slots.baseIconButton, which defines what an icon button should be.
  2. Modify the layout but keep default components with slots.tooltip + components like <QuickFilter />
  3. Modify the layout and design with slots.tooltip + the render function of <QuickFilter />

Unfortunately, since the fallback components can be rendered at runtime (i.e., bundlers can't know whether they're used or not), I think bundlers won't be able to tree-shake @mui/material even if users override all slots that use Material UI.

The base slots is an object that matches keys to components. Which are for now a wrapping around material components, which should allow the data grid to be somewhat indepenendt in terms of props API form the core team.

About tree-shaking, we can imagine to set the default value of the slots to be light in the unstiled package

In @base/x-charts

defaultslots= { baseIconButton: null }
// or
defaultslots= { baseIconButton: (props) => <button {...props} /> }

And in the one made to match with the core package

In @mui/x-charts

defaultslots= { baseIconButton: (props) => <MuiIconButton {...props} /> }

Such that @base/x-charts does not bundle the MuiIconButton and for the @mui/x-charts it does not matter, because if users whan to get rid of it they should prefer the @base/x-charts package

@bernardobelchior
Copy link
Member Author

If we look from a usecase perspective, you have:

  1. Using the same behavior and layout but different design with rootProps.slots.baseIconButton, which defines what an icon button should be.
  2. Modify the layout but keep default components with slots.tooltip + components like
  3. Modify the layout and design with slots.tooltip + the render function of

Yeah, I think that sums it up well.

Such that @base/x-charts does not bundle the MuiIconButton and for the @mui/x-charts it does not matter, because if users whan to get rid of it they should prefer the @base/x-charts package

Yeah, that could work. Another option that would be slightly cumbersome but wouldn't require a separate package would be to force users to register default slots in the App's entry point.

import { registerDefaultSlots } from '@mui/x-charts';
import materialDefaultSlots from '@mui/x-charts/material';

registerDefaultSlots(materialDefaultSlots);

I don't love that we would need to register the default slots in a single entry-point, so an alternative would be to follow the @testing-library/react and @testing-library/react/pure approach. By default, @mui/x-charts would register the default material slots. To avoid that, users would have to import from @mui/x-charts/pure, which wouldn't cause any side-effects.

However, this could cause some issues as well. For example, if you want to migrate from material slots, all imports would need to be updated, and incremental migration wouldn't be possible because having a single import from @mui/x-charts would be enough to register the material slots.

This was just an idea I wanted to write as it might spur some other idea.

@michelengelen michelengelen added new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 29, 2025
@KenanYusuf
Copy link
Member

KenanYusuf commented Apr 30, 2025

By default, @mui/x-charts would register the default material slots. To avoid that, users would have to import from @mui/x-charts/pure, which wouldn't cause any side-effects.

@bernardobelchior @alexfauquette it would be good to sync with @romgrk on how we are approaching the material/core packages for Data Grid. We should try to solve this problem in the same way across mui-x components.

@bernardobelchior
Copy link
Member Author

Thanks, @KenanYusuf! At the moment, I'm following the Data Grid's approach of slots + render props to maintain consistency. One thing I noticed is that it might be hard to tree-shake @mui/material because the slots are defaulted in runtime, which is why I was thinking of ideas to fix that. @romgrk do you have any thoughts on this?

@romgrk
Copy link
Contributor

romgrk commented Apr 30, 2025

The grid will have separate packages for the core grid and the material grid. The material grid can't tree-shake away @mui/material.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: charts This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants