[icons] Remove extraneous path

[timmydoza]

• I have followed (at least) the PR section of the contributing guide.

This pull request removes an invisible and extraneous <path>element from the MicOff icon. This <path> creates an invisible box around the whole icon. Other material-ui icons (including all other 'Mic' icons) don't have this box. This <path> can cause issues when trying to apply custom styles to the icon.

Example:

svg {
  stroke: red;
}

Result:

Expected:

[mui-pr-bot]

Details of bundle changes.

Comparing: 39c8170...7a7ca75

bundle	Size Change	Size	Gzip Change	Gzip
docs.landing	▼ -114 B (-0.19% )	60.4 kB	▼ -61 B (-0.37% )	16.4 kB
docs.main	▼ -96 B (-0.02% )	596 kB	▼ -43 B (-0.02% )	194 kB
@material-ui/core	--	361 kB	--	98.7 kB
@material-ui/core[umd]	--	317 kB	--	91.9 kB
@material-ui/lab	--	185 kB	--	55.3 kB
@material-ui/styles	--	51.4 kB	--	15.4 kB
@material-ui/system	--	14.8 kB	--	4.07 kB
Alert	--	84 kB	--	26.3 kB
AlertTitle	--	64.3 kB	--	20.3 kB
AppBar	--	64.1 kB	--	20.1 kB
Autocomplete	--	132 kB	--	41.4 kB
Avatar	--	65.4 kB	--	20.7 kB
AvatarGroup	--	62.4 kB	--	19.6 kB
Backdrop	--	68 kB	--	21 kB
Badge	--	65.5 kB	--	20.4 kB
BottomNavigation	--	62.6 kB	--	19.6 kB
BottomNavigationAction	--	75.7 kB	--	23.9 kB
Box	--	71 kB	--	21.7 kB
Breadcrumbs	--	67.9 kB	--	21.3 kB
Button	--	79.9 kB	--	24.5 kB
ButtonBase	--	74.2 kB	--	23.3 kB
ButtonGroup	--	83.4 kB	--	25.5 kB
Card	--	63 kB	--	19.7 kB
CardActionArea	--	75.3 kB	--	23.7 kB
CardActions	--	62.2 kB	--	19.5 kB
CardContent	--	62.1 kB	--	19.5 kB
CardHeader	--	65.2 kB	--	20.5 kB
CardMedia	--	62.5 kB	--	19.7 kB
Checkbox	--	83.2 kB	--	26.3 kB
Chip	--	82.8 kB	--	25.4 kB
CircularProgress	--	64.3 kB	--	20.3 kB
ClickAwayListener	--	3.91 kB	--	1.55 kB
Collapse	--	68.2 kB	--	21.1 kB
colorManipulator	--	3.88 kB	--	1.52 kB
Container	--	63.4 kB	--	19.8 kB
CssBaseline	--	57.7 kB	--	18.1 kB
Dialog	--	83.2 kB	--	25.9 kB
DialogActions	--	62.2 kB	--	19.5 kB
DialogContent	--	62.4 kB	--	19.6 kB
DialogContentText	--	64.2 kB	--	20.2 kB
DialogTitle	--	64.4 kB	--	20.2 kB
Divider	--	62.7 kB	--	19.7 kB
Drawer	--	85 kB	--	25.8 kB
ExpansionPanel	--	72.5 kB	--	22.7 kB
ExpansionPanelActions	--	62.2 kB	--	19.5 kB
ExpansionPanelDetails	--	62.1 kB	--	19.5 kB
ExpansionPanelSummary	--	78.3 kB	--	24.7 kB
Fab	--	77 kB	--	24 kB
Fade	--	23.4 kB	--	7.98 kB
FilledInput	--	73.7 kB	--	22.9 kB
FormControl	--	64.6 kB	--	20.2 kB
FormControlLabel	--	65.7 kB	--	20.7 kB
FormGroup	--	62.2 kB	--	19.5 kB
FormHelperText	--	63.5 kB	--	20 kB
FormLabel	--	63.6 kB	--	19.8 kB
Grid	--	65.3 kB	--	20.5 kB
GridList	--	62.6 kB	--	19.7 kB
GridListTile	--	63.9 kB	--	20 kB
GridListTileBar	--	63.4 kB	--	19.9 kB
Grow	--	24 kB	--	8.19 kB
Hidden	--	66.1 kB	--	20.8 kB
Icon	--	62.9 kB	--	19.8 kB
IconButton	--	76.3 kB	--	23.8 kB
Input	--	72.7 kB	--	22.7 kB
InputAdornment	--	65.3 kB	--	20.5 kB
InputBase	--	70.8 kB	--	22.2 kB
InputLabel	--	65.5 kB	--	20.2 kB
LinearProgress	--	65.5 kB	--	20.5 kB
Link	--	66.8 kB	--	21.1 kB
List	--	62.5 kB	--	19.5 kB
ListItem	--	77.3 kB	--	24.2 kB
ListItemAvatar	--	62.3 kB	--	19.5 kB
ListItemIcon	--	62.3 kB	--	19.5 kB
ListItemSecondaryAction	--	62.2 kB	--	19.5 kB
ListItemText	--	65.1 kB	--	20.5 kB
ListSubheader	--	62.9 kB	--	19.8 kB
Menu	--	89 kB	--	27.4 kB
MenuItem	--	78.4 kB	--	24.5 kB
MenuList	--	66.2 kB	--	20.7 kB
MobileStepper	--	68 kB	--	21.4 kB
Modal	--	14.5 kB	--	5.05 kB
NativeSelect	--	77 kB	--	24.3 kB
NoSsr	--	2.19 kB	--	1.04 kB
OutlinedInput	--	74.7 kB	--	23.3 kB
Paper	--	62.5 kB	--	19.5 kB
Popover	--	83.3 kB	--	25.8 kB
Popper	--	28.8 kB	--	10.3 kB
Portal	--	2.92 kB	--	1.3 kB
Radio	--	84.2 kB	--	26.5 kB
RadioGroup	--	64.6 kB	--	20.1 kB
Rating	--	70.7 kB	--	22.7 kB
RootRef	--	4.24 kB	--	1.64 kB
Select	--	117 kB	--	34.6 kB
Skeleton	--	63.1 kB	--	20 kB
Slide	--	25.5 kB	--	8.71 kB
Slider	--	76.8 kB	--	24.3 kB
Snackbar	--	75.6 kB	--	23.7 kB
SnackbarContent	--	63.7 kB	--	20.1 kB
SpeedDial	--	86.4 kB	--	27.2 kB
SpeedDialAction	--	119 kB	--	37.5 kB
SpeedDialIcon	--	64.7 kB	--	20.3 kB
Step	--	62.8 kB	--	19.7 kB
StepButton	--	82.5 kB	--	26.1 kB
StepConnector	--	62.9 kB	--	19.8 kB
StepContent	--	69.3 kB	--	21.7 kB
StepIcon	--	64.8 kB	--	20.2 kB
StepLabel	--	68.8 kB	--	21.7 kB
Stepper	--	65 kB	--	20.5 kB
styles/createMuiTheme	--	16.5 kB	--	5.81 kB
SvgIcon	--	63.2 kB	--	19.8 kB
SwipeableDrawer	--	92.4 kB	--	28.9 kB
Switch	--	82.3 kB	--	26 kB
Tab	--	76.5 kB	--	24.2 kB
Table	--	62.7 kB	--	19.7 kB
TableBody	--	62.3 kB	--	19.5 kB
TableCell	--	64.2 kB	--	20.2 kB
TableContainer	--	62.1 kB	--	19.5 kB
TableFooter	--	62.3 kB	--	19.5 kB
TableHead	--	62.3 kB	--	19.5 kB
TablePagination	--	144 kB	--	42 kB
TableRow	--	62.6 kB	--	19.7 kB
TableSortLabel	--	77.6 kB	--	24.4 kB
Tabs	--	85.8 kB	--	27.2 kB
TextareaAutosize	--	5.12 kB	--	2.14 kB
TextField	--	125 kB	--	36.6 kB
ToggleButton	--	76.3 kB	--	24.2 kB
ToggleButtonGroup	--	63.4 kB	--	20 kB
Toolbar	--	62.5 kB	--	19.6 kB
Tooltip	--	102 kB	--	32.4 kB
TreeItem	--	74.2 kB	--	23.5 kB
TreeView	--	66.8 kB	--	21.1 kB
Typography	--	63.8 kB	--	20 kB
useAutocomplete	--	14.6 kB	--	5.3 kB
useMediaQuery	--	2.58 kB	--	1.07 kB
Zoom	--	23.5 kB	--	8.1 kB

Generated by 🚫 dangerJS against 7a7ca75

[oliviertassinari]

Great catch! However, these icons are automatically generated, manual edits are not an option. We had to fix similar issues in the past. Do you think that you could try this different approach, from https://github.com/mui-org/material-ui/blob/bdebdfd4832c13a05463faeffef9687688be9715/packages/material-ui-icons/builder.js#L91-L99

Thanks!

[timmydoza]

Great catch! However, these icons are automatically generated, manual edits are not an option. We had to fix similar issues in the past. Do you think that you could try this different approach, from

https://github.com/mui-org/material-ui/blob/bdebdfd4832c13a05463faeffef9687688be9715/packages/material-ui-icons/builder.js#L91-L99

Thanks!

Gotcha. Thanks for the tip! I added the path to the noises array. Do I need to run any build scripts so that MicOff.js gets updated?

[oliviertassinari]

Do I need to run any build scripts so that MicOff.js gets updated?

This is going to be the challenging part! We haven't updated the icons for a few months. I can't guarantee the script still runs. The workflow is:

• yarn src:download: pull the SVG sources from Google
• yarn src:icons: convert the SVG icons into React components

[timmydoza]

Do I need to run any build scripts so that MicOff.js gets updated?

This is going to be the challenging part! We haven't updated the icons for a few months. I can't guarantee the script still runs. The workflow is:

• yarn src:download: pull the SVG sources from Google
• yarn src:icons: convert the SVG icons into React components

The scripts run, but they add about ten new icons and modify about 150+ more. Looks like some new kinds of 'noise' are added as well.

I committed the generated MicOff icon for now. I feel like including the other additions and modifications is a bit out of scope. What do you think?

Thanks again for the quick help!

[oliviertassinari]

I confirm, it runs without issue :). I have opened #19485 to sync the icons. I think that we could follow this workflow: 1. work on #19485 to get it on master. 2. #19483 rebased on top of master, commit all the changes, make sure we are good.

updated

[oliviertassinari]

I have tried a built-in option in SVGO, it seems to be fine. If you could have a second look, it would be awesome.

[timmydoza]

I have tried a built-in option in SVGO, it seems to be fine. If you could have a second look, it would be awesome.

Thanks! Nice approach. That seems to get rid of most of them, but there are still are about ten icons that have visible noise (with a stroke: property).

The icons that are left all have scale() in their paths. For instance: <path transform="scale(1.33, 1.33)" fill="none" d="M0 0h18v18H0z" /> from StarRate.js.

I'm trying to get rid of them with builder.js but no luck yet.

[oliviertassinari]

I gave up on them 😁. In the past, I remember having visual regressions linked to cleaning the icons too much. This would be my only concern, to make sure we don't alter the visual output of the icons.

[timmydoza]

I gave up on them grin. In the past, I remember having visual regressions linked to cleaning the icons too much. This would be my only concern, to make sure we don't alter the visual output of the icons.

Actually, looks like all the problem icons come from /legacy. Should these ten icons just be cleaned by hand?

[oliviertassinari]

Oh, smart, yes, please do :D.

[timmydoza]

Oh, smart, yes, please do :D.

Ok I cleaned some of the legacy icons. I think that's all of them!

One last thing that I want to point out - there are some icons in the /legacy directory that do have corresponding icons that are downloaded with yarn run src:download. They don't seem to actually be 'legacy', but the build script will overwrite new icons with legacy ones.

One simple idea is to add overwrite: false to line 241 of builder.js:
await fse.copy(path.join(__dirname, '/legacy'), options.outputDir, {overwrite: false});

This change updates 44 icons in /src because it's saving a newer version from google and not the version from /legacy.

[oliviertassinari]

@timmydoza Correct, I have added a step in the script to throw if it finds a duplicated icon.

[oliviertassinari]

Thank you, this initial one line change proposal has turned into something larger :)