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

[DataGrid] Input fields do not honor defined theme variant #2944

Closed
2 tasks done
sebastianfrey opened this issue Oct 22, 2021 · 8 comments
Closed
2 tasks done

[DataGrid] Input fields do not honor defined theme variant #2944

sebastianfrey opened this issue Oct 22, 2021 · 8 comments
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!

Comments

@sebastianfrey
Copy link
Contributor

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

When the DataGrid is a child of ThemeProvider with a custom theme, which overrides and sets the default MuiTextField variant to outlined, than the defined variant is not honored by the DataGrids input fields.

Expected behavior 🤔

Input fields rendered in the context of the DataGrid should honor theme overrides.

Steps to reproduce 🕹

Steps:

  1. Go to https://codesandbox.io/s/eager-cray-jz12w?file=/src/Demo.tsx
  2. Check the theme: TextField variant is set to outlined
  3. Click on the filter button or on the columns button.
  4. The rendered input fields are not styled with the outlined variant.

Context 🔦

I want to achieve a consistent look and feel in my application for my users.

Your environment 🌎

`npx @mui/envinfo`
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Order ID 💳 (optional)

22075

@sebastianfrey sebastianfrey added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 22, 2021
@DanailH
Copy link
Member

DanailH commented Oct 25, 2021

Hi, @sebastianfrey thanks for raising this. It is indeed a problem this is the problematic line -> https://github.com/mui-org/material-ui-x/blob/next/packages/grid/_modules_/grid/components/panel/filterPanel/GridFilterInputValue.tsx#L101

We need to rethink how we style some of the core components. One possibility is to have a grid-specific theme or just grab the theme and spread the props if there are any on to each core component we use?
Tagging @mui-org/x for ideas.

@DanailH DanailH added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 25, 2021
@sebastianfrey
Copy link
Contributor Author

sebastianfrey commented Oct 25, 2021

@DanailH thanks for looking into it. This line in the GridColumnsPanel is also problematic:

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

@DanailH
Copy link
Member

DanailH commented Oct 26, 2021

@sebastianfrey yes there are more than one cases where this problem will appear. We are discussing it internally how to handle the fix and what would be the best and most scalable approach.

@m4theushw
Copy link
Member

MUI Core has the same problem. Take as example the TablePagination component. If I want to change the variant of the inner Select I can't, since the hardcoded prop wins: https://codesandbox.io/s/tablepagination-material-demo-forked-w8oit?file=/demo.tsx

https://github.com/mui-org/material-ui/blob/b6cdaaf1a7f60de9ed66a491b7477fad19429905/packages/mui-material/src/TablePagination/TablePagination.js#L200-L201

As we discussed yesterday, here are the options I could think of:

  1. Get the default props from the theme and apply on MUI Core components where users might want to customize (e.g. TextField and Select):

    This option doesn't scale because there're other components that we use which also accept variants (FormControl is one). However, it works as users expect. If we choose this one, I would make it work only on those components that users ask.

    diff --git a/packages/grid/_modules_/grid/components/panel/GridColumnsPanel.tsx b/packages/grid/_modules_/grid/components/panel/GridColumnsPanel.tsx
    index 96f36c25..74019df2 100644
    --- a/packages/grid/_modules_/grid/components/panel/GridColumnsPanel.tsx
    +++ b/packages/grid/_modules_/grid/components/panel/GridColumnsPanel.tsx
    @@ -63,6 +63,7 @@ export function GridColumnsPanel() {
       const [searchValue, setSearchValue] = React.useState('');
       const ownerState = { classes: rootProps.classes };
       const classes = useUtilityClasses(ownerState);
    +  const textFieldProps = useThemeProps({ name: 'MuiTextField', props: {} });
     
       const toggleColumn = React.useCallback(
         (event: React.MouseEvent<HTMLButtonElement>) => {
    @@ -120,6 +121,7 @@ export function GridColumnsPanel() {
               onChange={handleSearchValueChange}
               variant="standard"
               fullWidth
    +          {...textFieldProps}
             />
           </GridPanelHeader>
           <GridPanelContent>
  2. Wrap the MUI Core components we use making the props coming from the theme to win. The default behavior of useThemeProps is to give priority to the props applied directly to the component, so the idea is to invert this. As an example, GridTextField could wrap TextField.

  3. Move the hardcoded variants to a grid-specific theme as pointed by @DanailH

  4. Add more slots to allow to pass props to inner components, encouraging users to compose their components.

  5. ?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 27, 2021

I would personally advocate for 4. or using default props when possible.

For instance:

  • this would get us one step closer to a DataGridUnstyled version of the component
  • we might not need variant="standard".
    It's already hard enough to have the grid panel with a great UX/UI (it's not the case yet) cc @danilo-leal. We would add more complexity by supporting different default values. Instead, we could leave the long tail of the use cases up to the developer to customize.

@DanailH
Copy link
Member

DanailH commented Nov 2, 2021

After a team discussion, we decided to proceed with option 4. For now, we will include slots for form elements. The naming convention we discussed using is Base* so for example the TextField will become BaseTextField.

@alexfauquette
Copy link
Member

Sounds this issue has been solved by @DanailH in #3490

The Text fields are customizable with

componentsProps={{
	baseTextField: {
		variant: "outlined",
		margin: "dense",
		size: "small"
	}
}}

https://codesandbox.io/s/sleepy-dubinsky-5iuf1o

Can we close this issue, or do we keep it open until we add a slot to each component from @mui/materil-ui has a slot?

@m4theushw
Copy link
Member

Closed by #3490

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

5 participants