Skip to content
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

[RFC] Make base components interchangeable #3066

Closed
m4theushw opened this issue Nov 1, 2021 · 7 comments
Closed

[RFC] Make base components interchangeable #3066

m4theushw opened this issue Nov 1, 2021 · 7 comments
Labels
discussion RFC Request For Comments

Comments

@m4theushw
Copy link
Member

m4theushw commented Nov 1, 2021

Problem

When components from MUI Core are used to compose other components in the DataGrid, in certain cases the variant or other prop is hardcoded to make them look correctly. This becomes a problem if the user wants to use a different variant (see #2944) or provide additional props by default. Currently, the default props added via theme doesn't work so there's no way to change how these components are rendered.

https://github.com/mui-org/material-ui-x/blob/cc5e8fa6f2ccc8dc3b2660d63c8ead7a440be59e/packages/grid/_modules_/grid/components/panel/GridColumnsPanel.tsx#L123

https://github.com/mui-org/material-ui-x/blob/cc5e8fa6f2ccc8dc3b2660d63c8ead7a440be59e/packages/grid/_modules_/grid/components/panel/filterPanel/GridFilterForm.tsx#L167

There's already the components and componentsProps which allow to override some components. However, these slots are only available for DataGrid's own components. As an example, to override the column selector in the filter panel, the user has to override the entire component.

This RFC wants to discuss how to shape the DataGrid props to allow the base components to be changed too. The natural path would be to add more slots, however, their names might collide (e.g. is components.Pagination the pagination component or the MUI component used to build the pagination component?). Also, making the base components interchangeable will allow us to provide a unstyled DataGrid which could be used with other design kits, e.g. AntDesign.

Proposed solution

  1. Add baseComponents and baseComponentsProps props. These props would have by default the MUI Core components that are used internally:

    import TextField from '@mui/material/TextField';
    
    <DataGrid baseComponents={{ TextField }} baseComponentsProps={{ textField: { variant: 'outlined' } }} />
  2. Add the base components inside components and componentsProps props:

    import TextField from '@mui/material/TextField';
    
    <DataGrid components={{ Base: { TextField } }} componentsProps={{ base: { textField: { variant: 'outlined' } } }} />
  3. Use a prefix to distinguish the base components from the slots:

    import TextField from '@mui/material/TextField';
    
    <DataGrid components={{ BaseTextField: TextField } }} componentsProps={{ baseTextField: { variant: 'outlined' } } }} />

Mentioning @michaldudak to help to propose other names.

@michaldudak
Copy link
Member

I'm not familiar with the structure of the DataGrid code. Could you explain in more detail what do these "base" components would be responsible for and how they differ from non-base ones? I.e. what would be the difference between setting BaseTextField: CustomTextField and TextField: CustomTextField?

@m4theushw
Copy link
Member Author

m4theushw commented Nov 2, 2021

@michaldudak The base components are any component from MUI Core used internally to compose other components, it can also be understood as the foundation we relay on.

https://github.com/mui-org/material-ui-x/blob/072b8f88339955340e156ec3d8c57a3b72941595/packages/grid/_modules_/grid/components/panel/filterPanel/GridFilterForm.tsx#L161

I.e. what would be the difference between setting BaseTextField: CustomTextField and TextField: CustomTextField?

TextField: CustomTextField would refer to the GridTextField component which could be a wrapper around BaseTextField.

Take as example the pagination rendered in the footer. Its slot is Pagination and the default value is the GridPagination component, which wraps MuiTablePagination. However, imagine that it wraps MuiPagination instead. How to change it if both will have the same name?

https://github.com/mui-org/material-ui-x/blob/072b8f88339955340e156ec3d8c57a3b72941595/packages/grid/_modules_/grid/components/GridPagination.tsx#L77

I used the "Mui" prefix to denote MUI Core components. In practical use, the confusion between whether it's a grid component or a base component is likely something users will find.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 2, 2021

+1 for option 3, as it aligns with the direction taken in #879 for the icons.

@DanailH
Copy link
Member

DanailH commented Nov 3, 2021

+1 for option 3 - my argument is that it will be easier to distinguish between core and grid-specific components.

@michaldudak
Copy link
Member

Yes, option 3 makes the most sense to me too.

@m4theushw
Copy link
Member Author

m4theushw commented Nov 8, 2021

We already have one slot for one of the components from the MUI Core: https://github.com/mui-org/material-ui-x/blob/95dd5d648fbeb05f3ce6fe9bd3464c451fe4a755/packages/grid/_modules_/grid/hooks/utils/useGridProcessedProps.ts#L60

To go with option 3, this has to be renamed to BaseCheckbox. This is a breaking change and it needs to be done before the stable release. Done in #3142

@m4theushw
Copy link
Member Author

These are the remaining components whose slot is missing:

  • CircularProgress
  • TablePagination
  • MenuList
  • MenuItem
  • Paper
  • Grow
  • IconButton
  • ListItemIcon
  • InputBase
  • InputLabel
  • FormControlLabel
  • Box
  • Badge
  • NoSsr
  • ClickAwayListener
  • Grow
  • Divider

@oliviertassinari oliviertassinari added the RFC Request For Comments label Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion RFC Request For Comments
Projects
None yet
Development

No branches or pull requests

4 participants