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

[Button] Improve horizontal padding #17640

Merged
merged 5 commits into from Oct 1, 2019
Merged

Conversation

mbrookes
Copy link
Member

@mbrookes mbrookes commented Sep 30, 2019

Closes #17634

In addition to apply the change discussed in #17634, it fixes some inconsistencies for small and large sizes with text and outlined variants, and adds text for these changes.

Arguably a breaking change for:

  • Layout (minor button size changes)
  • Customization (class name changes)

@mbrookes mbrookes added the component: button This is the name of the generic UI component, not the React module! label Sep 30, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Sep 30, 2019

@material-ui/core: parsed: +0.11% , gzip: +0.09%

Details of bundle changes.

Comparing: 05c533b...403c281

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.11% 🔺 +0.09% 🔺 332,538 332,913 90,732 90,815
@material-ui/core/Paper 0.00% 0.00% 68,935 68,935 20,541 20,541
@material-ui/core/Paper.esm 0.00% 0.00% 62,372 62,372 19,275 19,275
@material-ui/core/Popper 0.00% 0.00% 28,405 28,405 10,172 10,172
@material-ui/core/Textarea 0.00% 0.00% 5,082 5,082 2,136 2,136
@material-ui/core/TrapFocus 0.00% 0.00% 3,766 3,766 1,596 1,596
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,348 16,348 5,818 5,818
@material-ui/core/useMediaQuery 0.00% 0.00% 2,488 2,488 1,050 1,050
@material-ui/lab 0.00% 0.00% 143,405 143,405 43,808 43,808
@material-ui/styles 0.00% 0.00% 51,694 51,694 15,364 15,364
@material-ui/system 0.00% 0.00% 15,581 15,581 4,341 4,341
Button +0.48% 🔺 +0.27% 🔺 78,772 79,147 24,043 24,109
Modal 0.00% 0.00% 14,542 14,542 5,113 5,113
Portal 0.00% 0.00% 2,907 2,907 1,318 1,318
Rating 0.00% 0.00% 70,238 70,238 21,929 21,929
Slider 0.00% 0.00% 75,379 75,379 23,260 23,260
colorManipulator 0.00% 0.00% 3,835 3,835 1,519 1,519
docs.landing 0.00% 0.00% 54,267 54,267 14,344 14,344
docs.main +0.06% 🔺 +0.04% 🔺 588,062 588,437 188,043 188,120
packages/material-ui/build/umd/material-ui.production.min.js +0.12% 🔺 +0.08% 🔺 303,356 303,722 87,276 87,350

Generated by 🚫 dangerJS against 403c281

@oliviertassinari

This comment has been minimized.

@oliviertassinari
Copy link
Member

@mbrookes The padding changes look great!

I see a way to work around the customization breaking change I couldn't see previously 🤦‍♂️. I'm adding another commit for it.

Regarding #17638, do you think that we should give him the credit or continue here?

@mbrookes

This comment has been minimized.

@mbrookes
Copy link
Member Author

do you think that we should give him the credit or continue here?

All the same once it's merged, so whichever you prefer.

@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 30, 2019

Customization (class name changes)

This one should be addressed.

Layout (minor button size changes)

The master branch has 3 other changes (MenuItem min-height change, Table spec update and Chip spec update) that have a similar impact. I'm OK if we release it in the next version (v4.5.0). I can't think of any worth-case impact that would break people application. Still, we need to be cautious.

@oliviertassinari oliviertassinari changed the title [Button] Fix sizes [Button] Improve vertical padding Sep 30, 2019
@mbrookes
Copy link
Member Author

I can't think of any worst-case impact that would break peoples application

Neither can I. I think it should be safe enough.

@oliviertassinari oliviertassinari merged commit 09ec808 into mui:master Oct 1, 2019
@mbrookes mbrookes deleted the button-sizes branch October 1, 2019 10:23
@oliviertassinari oliviertassinari changed the title [Button] Improve vertical padding [Button] Improve horizontal padding Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: button This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bigger horizontal padding for small size button
3 participants