[IconButton] Add a size prop

[leMaik]

This PR adds a size property to the IconButton component. Setting it to small applies density as described in the Material Design Guidelines:

Regarding the demo: The delete icon looked ugly in the small size, but the arrow looks fine.

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

[mui-pr-bot]

Details of bundle changes.

Comparing: c0287df...75b08eb

bundle	parsed diff	gzip diff	prev parsed	current parsed	prev gzip	current gzip
@material-ui/core	+0.07% 🔺	+0.06% 🔺	371,600	371,871	91,876	91,927
@material-ui/core/Paper	0.00%	0.00%	76,930	76,930	19,372	19,372
@material-ui/core/Paper.esm	0.00%	0.00%	71,601	71,601	18,779	18,779
@material-ui/core/Popper	0.00%	0.00%	30,462	30,462	10,582	10,582
@material-ui/core/styles/createMuiTheme	0.00%	0.00%	17,286	17,286	5,717	5,717
@material-ui/core/useMediaQuery	0.00%	0.00%	2,486	2,486	1,048	1,048
@material-ui/lab	+0.04% 🔺	+0.02% 🔺	184,505	184,571	50,732	50,740
@material-ui/styles	0.00%	0.00%	57,720	57,720	16,273	16,273
@material-ui/system	0.00%	0.00%	17,062	17,062	4,487	4,487
Button	0.00%	0.00%	99,633	99,633	26,634	26,634
Modal	0.00%	0.00%	98,713	98,713	26,171	26,171
colorManipulator	0.00%	0.00%	3,232	3,232	1,296	1,296
docs.landing	0.00%	0.00%	52,369	52,369	11,488	11,488
docs.main	+0.03% 🔺	+0.03% 🔺	678,827	679,047	206,232	206,291
packages/material-ui/build/umd/material-ui.production.min.js	+0.08% 🔺	+0.05% 🔺	322,256	322,516	85,046	85,085

Generated by 🚫 dangerJS against 75b08eb

[oliviertassinari]

@leMaik I gave this pull request a go. I'm proposing the following changes:

• Use the same size strategy for the IconButton that we use for the Fab and the Button components. We talk about it before. I think that we can revisit the approach. But until then, I think that it's better to use the same strategy. However, the same strategy doesn't really mean anything in this context as it's a mess right now in the codebase, it's the same strategy, at least for the size property. If you are looking for a topic to address :), it's a good one, I wish we can do it in a single pass.
• I have changed the size style a bit to match icon.sizeSmall. I'm not sure about this change.
• I have updated the fontSize to use our theme.typography.pxToRem helper
• I have updated the tests to use the mount API
• I have updated the TypeScript definitions class API

[leMaik]

@oliviertassinari Awesome! 🎉 Anything left to be done?

I have changed the size style a bit to match icon.sizeSmall. I'm not sure about this change.

The specs say 18px. I don't think we should use 20px then. The intent of this PR was to match the specs in the first place. Also, 20px + 6px total padding = 26px, which is not a very Material-Design-y number.

At the moment, I only have time for Material-UI during the weekend but I try to keep the PRs coming.

If you are looking for a topic to address :), it's a good one, I wish we can do it in a single pass.

That sounds like a good weekend-thing 😄

[oliviertassinari]

@leMaik As long as you can have fun during your weekends on Material-UI, it's 👌. We already have new conflicts 😆. My original motivation for the font size change was to reduce the font size cardinality. The less different values we have, the most consistent the UI should be. In this case, maybe the right move is to do the opposite, change the icon font size small value?

[leMaik]

We already have new conflicts 😆.

@oliviertassinari Rebase ✔️ I changed the font size back to 18px, let's stick to the specs then. Some icons, e.g. the arrow, look good at 18px. 18px is also the size of the table sorting label (according to the specs and my previous PR).

[oliviertassinari]

It might looks closer to the specification with padding 2px and font-size 20px:

[leMaik]

@oliviertassinari The specs pretty clearly say padding=3, size=18. It looks fine to me.

[oliviertassinari]

@leMaik I'm comparing the visual output, the specification values assume a lot about how the icons are positioned within the available space. The material svg icons are wasting a lot of space:

the background color fit the icon size, no padding, no margin

[leMaik]

@oliviertassinari In the table specs, the wasted space of the icon is included in the 18px (18px including the wasted space). I'd assume that it's the same here. That said, I don't see a point in a long debate about two pixels of size 😅

[oliviertassinari]

@leMaik Ok cool, what do you think of making the svg icon and font icon small font size be 18px?

[leMaik]

@oliviertassinari Changing the small size would break people's UIs. 😱

[oliviertassinari]

One pixel can change how the browser is rasterizing the svg, take the bin icon, 18px is blurred when 20px isn't (on my Machine)

20px vs 18px

[oliviertassinari]

It's a major, wouldn't it be better to have only 3 icon font size values than 4?

[leMaik]

@oliviertassinari An icon is more like a building block. I didn't ever use it as a standalone component and never needed a size property. Most of the time, I use it inside other components that do the sizing for me or I use custom styles to set the size precisely anyway. Do we even need a size property for icons?