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] Changing GridToolbarFilterButton variant #10409

Closed
2 tasks done
Tracked by #7902
SammyGutierrez335 opened this issue Sep 20, 2023 · 4 comments · Fixed by #11357
Closed
2 tasks done
Tracked by #7902

[datagrid] Changing GridToolbarFilterButton variant #10409

SammyGutierrez335 opened this issue Sep 20, 2023 · 4 comments · Fixed by #11357
Assignees
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! customization: css Design CSS customizability plan: Premium Impact at least one Premium user v7.x

Comments

@SammyGutierrez335
Copy link

SammyGutierrez335 commented Sep 20, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Steps:

  1. Set up a data grid in a typescript file using the MUI documentation. https://mui.com/x/react-data-grid/
  2. In your component, add a slots prop and pass {toolbar: CustomToolbar}
  3. set up a CustomToolbar function that returns the code below:
<GridToolbarContainer>
  <GridToolbarColumnsButton variant="outlined" />
  <GridToolbarFilterButton variant="outlined"/>
  <GridToolbarDensitySelector variant="outlined" />
  <GridToolbarExport variant="outlined"/>
</GridToolbarContainer>

snippet of actual code in my project
image

Current behavior 😯

The Typescript compiler/IDE highlights the 'variant' prop on the GridToolbarFilterButton as an invalid prop but renders correctly. Changing the variant prop will change the look of the button as expected.
The other buttons in this custom toolbar also contain the same variant prop but do not throw an error

Expected behavior 🤔

I expect the GridToolbarFilterButton to act like the other buttons and accept the variant prop without triggering Typescript compiling error or for the data grid component to reject the prop (and varient changes) if it truly is an invalid prop.

Context 🔦

I am implementing a data grid to show various user data to supervisors.
I would like all buttons to share the same 'outlined' styling/hover effects (leveraging the prop) but if I do, the typescript error throw will impact development because each time my application renders I am forced to close out the error. This is also a block from running a build because my typescript config is setup to prevent it. My work around is to remove the prop during development/building and just informing my team that mismatch styling will hopefully be addressed later.

Your environment 🌎

npx @mui/envinfo
  npmPackages:
    @emotion/react: ^11.11.1 => 11.11.1
    @emotion/styled: ^11.11.0 => 11.11.0
    @mui/base:  5.0.0-beta.16
    @mui/core-downloads-tracker:  5.14.10
    @mui/icons-material: ^5.14.3 => 5.14.9
    @mui/lab: ^5.0.0-alpha.143 => 5.0.0-alpha.145
    @mui/material: ^5.14.10 => 5.14.10
    @mui/private-theming:  5.14.10
    @mui/styled-engine:  5.14.10
    @mui/system: ^5.14.5 => 5.14.10
    @mui/types:  7.2.4
    @mui/utils:  5.14.10
    @mui/x-data-grid:  6.14.0
    @mui/x-data-grid-premium: ^6.14.0 => 6.14.0
    @mui/x-data-grid-pro:  6.14.0
    @mui/x-date-pickers: ^6.12.1 => 6.14.0
    @mui/x-date-pickers-pro: ^6.12.1 => 6.14.0
    @mui/x-license-pro:  6.10.2
    @mui/x-tree-view:  6.0.0-alpha.1
    @types/react: ^18.2.21 => 18.2.22
    react: ^18.2.0 => 18.2.0
    react-dom: ^18.2.0 => 18.2.0
    typescript: ^4.9.5 => 4.9.5
    
    browser: google chrome

Order ID or Support key 💳 (optional)

No response

@SammyGutierrez335 SammyGutierrez335 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 20, 2023
@zannager zannager added component: data grid This is the name of the generic UI component, not the React module! plan: Premium Impact at least one Premium user labels Sep 20, 2023
@cherniavskii
Copy link
Member

Hi @SammyGutierrez335
Please try this instead:

- <GridToolbarFilterButton variant="outlined" />
+ <GridToolbarFilterButton componentsProps={{ button: { variant: 'outlined' } }} />

The filter button is wrapped with the tooltip, so the props are a bit different.

@cherniavskii
Copy link
Member

We can improve the consistency in v7.
Why is the filter button the only one wrapped into a tooltip? Should we remove it? Or add it to other buttons as well?

@cherniavskii cherniavskii added breaking change v7.x and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 22, 2023
@cherniavskii cherniavskii changed the title DataGridPremium GridToolbarFilterButton component with variant prop renders properly but throws 'variant prop does not exist on type IntrinsicAttributes & Omit<GridToolbarFilterButtonProps, "ref"> & RefAttributes<HTMLButtonElement>' error [datagrid] Changing GridToolbarFilterButton variant Sep 22, 2023
@cherniavskii cherniavskii added the customization: css Design CSS customizability label Sep 22, 2023
@SammyGutierrez335
Copy link
Author

SammyGutierrez335 commented Sep 22, 2023

Thank you! That took care of the outlining/error issue.

As mentioned, now there is a tooltip exclusively only on this button which is not ideal but at least the buttons look uniform.
I would opt to remove the tooltip if the text isn't customizable. If there is a work around to disable the tooltip that would be greatly appreciated, if not, I'll wait for the v7 changes.

Thanks again. You all are awesome!

@cherniavskii
Copy link
Member

cherniavskii commented Oct 24, 2023

The decision is to:

  • wrap every button in the toolbar into a Tooltip
  • accept tooltip props through slotProps.tooltip to avoid confusion of props passed to GridToolbarFIlterButton being forwarded to the tooltip:
- <GridToolbarFilterButton title="Custom tooltip title" />
+ <GridToolbarFilterButton slotProps={{ tooltip: { title: "Custom tooltip title" } }} />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! customization: css Design CSS customizability plan: Premium Impact at least one Premium user v7.x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants