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

[Icon] Change default fontSize name #14993

Closed

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Mar 21, 2019

Deprecation

Few people should be affected by this change as it focuses on the default value. Most people don't specify the default value, and if you have specified a wrong value, it will only warn, the output should be the same.

-<Icon size="default" />
+<Icon size="medium" />
-<SvgIcon size="default" />
+<SvgIcon size="medium" />

Why? It's for consistency:
https://github.com/mui-org/material-ui/blob/d1a7d76db79b2f1f03068c7c3bc0c9f28a7c5a64/packages/material-ui/src/Button/Button.js#L292
https://github.com/mui-org/material-ui/blob/d1a7d76db79b2f1f03068c7c3bc0c9f28a7c5a64/packages/material-ui/src/TableCell/TableCell.js#L203

@oliviertassinari oliviertassinari added breaking change deprecation New deprecation message component: Icon The React component. component: SvgIcon The React component. and removed breaking change deprecation New deprecation message labels Mar 21, 2019
@eps1lon eps1lon mentioned this pull request Mar 21, 2019
56 tasks
@mui-pr-bot
Copy link

mui-pr-bot commented Mar 21, 2019

Details of bundle changes.

Comparing: d1a7d76...7263762

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core -0.00% -0.01% 352,929 352,927 90,642 90,633
@material-ui/core/Paper 0.00% 0.00% 68,626 68,626 19,974 19,974
@material-ui/core/Paper.esm 0.00% 0.00% 62,358 62,358 19,075 19,075
@material-ui/core/Popper 0.00% -0.01% 30,460 30,460 10,528 10,527
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 17,384 17,384 5,726 5,726
@material-ui/core/useMediaQuery 0.00% 0.00% 2,469 2,469 1,037 1,037
@material-ui/lab -0.00% 0.00% 152,181 152,180 44,533 44,535
@material-ui/styles 0.00% 0.00% 53,837 53,837 15,613 15,613
@material-ui/system 0.00% 0.00% 17,138 17,138 4,520 4,520
Button 0.00% 0.00% 90,918 90,918 26,970 26,970
Modal 0.00% -0.00% 84,712 84,712 25,262 25,261
colorManipulator 0.00% 0.00% 3,232 3,232 1,300 1,300
docs.landing 0.00% 0.00% 51,843 51,843 11,349 11,349
docs.main 0.00% 0.00% 647,762 647,773 201,135 201,138
packages/material-ui/build/umd/material-ui.production.min.js -0.00% 0.00% 301,958 301,956 83,730 83,730

Generated by 🚫 dangerJS against 7263762

@eps1lon
Copy link
Member

eps1lon commented Mar 21, 2019

Can't we just alias default to medium for now and add a deprecation message in the prop types validator?

@oliviertassinari oliviertassinari added deprecation New deprecation message and removed breaking change labels Mar 21, 2019
@oliviertassinari
Copy link
Member Author

@eps1lon OK

@oliviertassinari oliviertassinari added this to the v4.1 milestone Mar 22, 2019
@oliviertassinari oliviertassinari added on hold There is a blocker, we need to wait and removed PR: good for merge labels Mar 22, 2019
@oliviertassinari oliviertassinari changed the base branch from next to master May 23, 2019 21:10
@oliviertassinari
Copy link
Member Author

I'm closing for the moment. I will come back to it later, probably in 4-6 months to prepare v5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Icon The React component. component: SvgIcon The React component. deprecation New deprecation message on hold There is a blocker, we need to wait
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants