mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.35k stars 1.3k forks source link

[SvgIcon] wrong css width/height attributes #2331

Closed cedricb-obs closed 1 year ago

cedricb-obs commented 3 years ago

Current Behavior 😯

When i use DataGrid with Toolbar component, the Filters icon size isn't the same as the others.

Expected Behavior 🤔

Same sizes of the toolbar icons.

Steps to Reproduce 🕹

Sample code:

import { DataGrid, GridToolbar } from '@material-ui/data-grid';

const MyTable = ({ columns, rows }) => (<DataGrid columns={columns} rows={rows} components={{ Toolbar: GridToolbar }} />)

Your Environment 🌎

Tested with Chrome Version 92.0.4515.131 (Build officiel) (64 bits)

Solution

Use rem instead of em

Captures

mui-svgicon-filters-size.png mui-svgicon-density-size.png mui-svgicon-quickfix.png

cedricb-obs commented 3 years ago

@eps1lon hi ✋ why you transferred my issue inside this project ? 🤔

i pushed a quickfix with the initial issue to switch SvgIcon css witdh/height to rem instead of em

With the transfer, we lost the fix and DataGrid Toolbar component work properly, so i don't understand..

cedricb-obs commented 3 years ago

ok u commented my PR in same time.. 🙄 😉

m4theushw commented 3 years ago

Could you provide a live example? This codesandbox.io template may be a good starting point.

Looking at the demo in https://material-ui.com/components/data-grid/demo/ the icons are at the correct size:

image
cedricb-obs commented 3 years ago

Could you provide a live example? This codesandbox.io template may be a good starting point.

Looking at the demo in https://material-ui.com/components/data-grid/demo/ the icons are at the correct size:

image

You can see sample here and after more research, it seems to come from another deps (boosted and/or admin-lte) CSS conflict.
Sorry for the inconvenience.. but i think using rem is better than em 🤔 😜

cedricb-obs commented 3 years ago

i add another info, in this GridToolbar, all icons are in 18x18 px except this Filter icon (24x24).
i think this is the origin of my pbm.. not same sizes with em result in somes random behavior 🤔

oliviertassinari commented 3 years ago

Ok, so we only need to fix the font size to be 18x18 and we are good?

diff --git a/packages/grid/_modules_/grid/components/toolbar/GridToolbarFilterButton.tsx b/packages/grid/_modules_/grid/components/toolbar/GridToolbarFilterButton.tsx
index ff5439e7..838572b4 100644
--- a/packages/grid/_modules_/grid/components/toolbar/GridToolbarFilterButton.tsx
+++ b/packages/grid/_modules_/grid/components/toolbar/GridToolbarFilterButton.tsx
@@ -115,7 +115,7 @@ export const GridToolbarFilterButton = React.forwardRef<
         aria-label={apiRef!.current.getLocaleText('toolbarFiltersLabel')}
         startIcon={
           <Badge badgeContent={counter} color="primary">
-            <OpenFilterButtonIcon />
+            <OpenFilterButtonIcon fontSize="inherit" />
           </Badge>
         }
         {...buttonProps}
m4theushw commented 3 years ago

Ok, so we only need to fix the font size to be 18x18 and we are good?

The filter icon becomes too small with fontSize="inherit". The problem with this icon in particular is that it has more padding than the others.

image

Sorry for the inconvenience.. but i think using rem is better than em 🤔 😜

You can override the icons to use a custom one with the size you want: https://material-ui.com/components/data-grid/components/#icons

oliviertassinari commented 3 years ago

@m4theushw I didn't shock me, I guess we can explore other options.

cherniavskii commented 1 year ago

The icons indeed might have a different size, but it looks fine in v6:

We don't have any actions to take here.