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

[List] Update to match the specification #15339

Merged
merged 6 commits into from
Apr 15, 2019

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Apr 13, 2019

Closes #6098
Closes #14752 the padding is identical: 16px.

Breaking changes

Rework the list components to match the specification:

  • The use of the ListItemAvatar component is required when using an avatar
  • The use of the ListItemIcon component is required when using a left checkbox
  • The edge property should be set on the icon buttons.

@oliviertassinari oliviertassinari added breaking change component: list This is the name of the generic UI component, not the React module! labels Apr 13, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Apr 13, 2019

@material-ui/core: parsed: -0.23% 😍, gzip: -0.16% 😍

Details of bundle changes.

Comparing: 71d4ebe...af3b825

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core -0.23% -0.16% 316,385 315,642 85,502 85,365
@material-ui/core/Paper 0.00% 0.00% 67,277 67,277 19,975 19,975
@material-ui/core/Paper.esm 0.00% 0.00% 60,639 60,639 18,883 18,883
@material-ui/core/Popper 0.00% 0.00% 34,906 34,906 11,861 11,861
@material-ui/core/Textarea 0.00% 0.00% 5,866 5,866 2,465 2,465
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 15,898 15,898 5,773 5,773
@material-ui/core/useMediaQuery 0.00% 0.00% 2,106 2,106 974 974
@material-ui/lab 0.00% 0.00% 144,207 144,207 43,539 43,539
@material-ui/styles 0.00% 0.00% 50,831 50,831 15,020 15,020
@material-ui/system 0.00% 0.00% 11,765 11,765 3,929 3,929
Button 0.00% 0.00% 88,247 88,247 26,501 26,501
Modal 0.00% 0.00% 82,524 82,524 24,821 24,821
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 50,908 50,908 11,210 11,210
docs.main -0.06% -0.03% 647,485 647,091 202,008 201,955
packages/material-ui/build/umd/material-ui.production.min.js -0.23% -0.17% 292,961 292,276 82,472 82,329

Generated by 🚫 dangerJS against af3b825

@oliviertassinari oliviertassinari force-pushed the list-specification branch 2 times, most recently from 479b58e to 3992309 Compare April 13, 2019 17:49
@oliviertassinari oliviertassinari added this to the v4 milestone Apr 13, 2019
@oliviertassinari oliviertassinari force-pushed the list-specification branch 3 times, most recently from f143cc4 to 32e5846 Compare April 13, 2019 18:25
@oliviertassinari oliviertassinari marked this pull request as ready for review April 13, 2019 18:30
@oliviertassinari oliviertassinari force-pushed the list-specification branch 2 times, most recently from e23f166 to 14dc2be Compare April 14, 2019 15:06
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's some nice cleanup

docs/src/pages/demos/app-bar/BottomAppBar.js Show resolved Hide resolved
docs/src/pages/demos/lists/AlignItemsList.tsx Outdated Show resolved Hide resolved
docs/src/pages/demos/lists/CheckboxListSecondary.js Outdated Show resolved Hide resolved
packages/material-ui/src/ListItem/ListItem.js Show resolved Hide resolved
@oliviertassinari oliviertassinari merged commit 13ac4ce into mui:next Apr 15, 2019
/* Styles applied to the root element. */
root: {
flex: '1 1 auto',
minWidth: 0,
padding: '0 16px',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oliviertassinari We are migrating v3 -> v4 and here and there paddings get removed breaking some layout
is there any particular reason why these changes were applied?
Do we know how many components undergone such style adjustments?

Copy link
Member Author

@oliviertassinari oliviertassinari Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember, nor I have the time to look at it. We encourage to have visual regression tests when upgrading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: list 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.

5 participants