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

[Container] Add disableGutters prop #15872

Merged

Conversation

divyanshutomar
Copy link
Contributor

Closes #15708

@mui-pr-bot
Copy link

mui-pr-bot commented May 26, 2019

Details of bundle changes.

Comparing: a973b2f...75f0d16

bundle Size Change Size Gzip Change Gzip
Container ▲ +118 B (+0.19% ) 62.1 kB ▲ +43 B (+0.22% ) 19.3 kB
@material-ui/core[umd] ▲ +118 B (+0.04% ) 312 kB ▲ +33 B (+0.04% ) 90.1 kB
docs.main ▲ +118 B (+0.02% ) 610 kB ▲ +27 B (+0.01% ) 194 kB
@material-ui/core ▲ +118 B (+0.03% ) 355 kB ▲ +25 B (+0.03% ) 97.2 kB
@material-ui/lab -- 174 kB -- 52.3 kB
@material-ui/styles -- 51 kB -- 15.3 kB
@material-ui/system -- 14.8 kB -- 4.06 kB
AppBar -- 62.7 kB -- 19.5 kB
Autocomplete -- 128 kB -- 39.8 kB
Avatar -- 64.2 kB -- 20.1 kB
Backdrop -- 66.6 kB -- 20.5 kB
Badge -- 64.2 kB -- 19.9 kB
BottomNavigation -- 61.3 kB -- 19.1 kB
BottomNavigationAction -- 74.3 kB -- 23.4 kB
Box -- 69.6 kB -- 21 kB
Breadcrumbs -- 66.9 kB -- 20.9 kB
Button -- 78.3 kB -- 23.9 kB
ButtonBase -- 72.8 kB -- 22.8 kB
ButtonGroup -- 80.9 kB -- 24.8 kB
Card -- 61.6 kB -- 19.2 kB
CardActionArea -- 73.9 kB -- 23.2 kB
CardActions -- 60.9 kB -- 19 kB
CardContent -- 60.9 kB -- 19 kB
CardHeader -- 64 kB -- 20 kB
CardMedia -- 61.2 kB -- 19.2 kB
Checkbox -- 80.8 kB -- 25.4 kB
Chip -- 81.4 kB -- 24.8 kB
CircularProgress -- 63 kB -- 19.8 kB
ClickAwayListener -- 3.87 kB -- 1.56 kB
Collapse -- 66.8 kB -- 20.6 kB
colorManipulator -- 3.85 kB -- 1.52 kB
CssBaseline -- 56.5 kB -- 17.6 kB
Dialog -- 81.5 kB -- 25.4 kB
DialogActions -- 61 kB -- 19 kB
DialogContent -- 61.1 kB -- 19.1 kB
DialogContentText -- 62.9 kB -- 19.7 kB
DialogTitle -- 63.2 kB -- 19.7 kB
Divider -- 61.5 kB -- 19.2 kB
docs.landing -- 52.4 kB -- 11.4 kB
Drawer -- 83.2 kB -- 25.2 kB
ExpansionPanel -- 70.2 kB -- 21.8 kB
ExpansionPanelActions -- 61 kB -- 19 kB
ExpansionPanelDetails -- 60.8 kB -- 19 kB
ExpansionPanelSummary -- 76.9 kB -- 24.2 kB
Fab -- 75.6 kB -- 23.5 kB
Fade -- 22.5 kB -- 7.71 kB
FilledInput -- 72.4 kB -- 22.4 kB
FormControl -- 63.3 kB -- 19.6 kB
FormControlLabel -- 64.4 kB -- 20.1 kB
FormGroup -- 60.9 kB -- 19 kB
FormHelperText -- 62.2 kB -- 19.4 kB
FormLabel -- 62.4 kB -- 19.2 kB
Grid -- 64 kB -- 20 kB
GridList -- 61.4 kB -- 19.2 kB
GridListTile -- 62.6 kB -- 19.5 kB
GridListTileBar -- 62.1 kB -- 19.4 kB
Grow -- 23.1 kB -- 7.83 kB
Hidden -- 64.8 kB -- 20.2 kB
Icon -- 61.7 kB -- 19.2 kB
IconButton -- 75 kB -- 23.3 kB
Input -- 71.3 kB -- 22.1 kB
InputAdornment -- 64 kB -- 20 kB
InputBase -- 69.4 kB -- 21.7 kB
InputLabel -- 64.2 kB -- 20 kB
LinearProgress -- 64.2 kB -- 20 kB
Link -- 65.5 kB -- 20.6 kB
List -- 61.3 kB -- 19 kB
ListItem -- 76 kB -- 23.6 kB
ListItemAvatar -- 61 kB -- 19 kB
ListItemIcon -- 61.1 kB -- 19 kB
ListItemSecondaryAction -- 60.9 kB -- 19 kB
ListItemText -- 63.9 kB -- 19.9 kB
ListSubheader -- 61.7 kB -- 19.3 kB
Menu -- 87.2 kB -- 26.8 kB
MenuItem -- 77 kB -- 23.9 kB
MenuList -- 64.9 kB -- 20.2 kB
MobileStepper -- 66.6 kB -- 20.8 kB
Modal -- 14.3 kB -- 5 kB
NativeSelect -- 75.6 kB -- 23.7 kB
NoSsr -- 2.19 kB -- 1.03 kB
OutlinedInput -- 72.8 kB -- 22.6 kB
Paper -- 61.1 kB -- 19 kB
Popover -- 81.5 kB -- 25.1 kB
Popper -- 28.7 kB -- 10.2 kB
Portal -- 2.9 kB -- 1.3 kB
Radio -- 81.7 kB -- 25.7 kB
RadioGroup -- 62.1 kB -- 19.4 kB
Rating -- 68.9 kB -- 22.1 kB
RootRef -- 4.43 kB -- 1.67 kB
Select -- 113 kB -- 33.5 kB
Skeleton -- 61.5 kB -- 19.3 kB
Slide -- 24.5 kB -- 8.33 kB
Slider -- 74.5 kB -- 23.3 kB
Snackbar -- 76 kB -- 23.7 kB
SnackbarContent -- 64.5 kB -- 20.2 kB
SpeedDial -- 84.9 kB -- 26.7 kB
SpeedDialAction -- 116 kB -- 36.5 kB
SpeedDialIcon -- 63.5 kB -- 19.9 kB
Step -- 61.5 kB -- 19.2 kB
StepButton -- 81.1 kB -- 25.5 kB
StepConnector -- 61.6 kB -- 19.3 kB
StepContent -- 67.9 kB -- 21.2 kB
StepIcon -- 63.5 kB -- 19.7 kB
StepLabel -- 67.5 kB -- 21.1 kB
Stepper -- 63.6 kB -- 20 kB
styles/createMuiTheme -- 15.7 kB -- 5.47 kB
SvgIcon -- 61.9 kB -- 19.3 kB
SwipeableDrawer -- 90.6 kB -- 28 kB
Switch -- 80 kB -- 25 kB
Tab -- 75.2 kB -- 23.7 kB
Table -- 61.5 kB -- 19.2 kB
TableBody -- 61 kB -- 19 kB
TableCell -- 62.9 kB -- 19.7 kB
TableFooter -- 61 kB -- 19 kB
TableHead -- 61 kB -- 19 kB
TablePagination -- 140 kB -- 40.8 kB
TableRow -- 61.4 kB -- 19.1 kB
TableSortLabel -- 76.2 kB -- 23.9 kB
Tabs -- 84.3 kB -- 26.5 kB
TextareaAutosize -- 5.06 kB -- 2.11 kB
TextField -- 122 kB -- 35.6 kB
ToggleButton -- 75 kB -- 23.7 kB
ToggleButtonGroup -- 62.1 kB -- 19.4 kB
Toolbar -- 61.2 kB -- 19.1 kB
Tooltip -- 99.7 kB -- 31.5 kB
TreeItem -- 72.5 kB -- 22.8 kB
TreeView -- 65.3 kB -- 20.4 kB
Typography -- 62.5 kB -- 19.5 kB
useAutocomplete -- 12.6 kB -- 4.63 kB
useMediaQuery -- 2.47 kB -- 1.04 kB
Zoom -- 22.5 kB -- 7.71 kB

Generated by 🚫 dangerJS against 75f0d16

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

This looks great but I have one concern with the breaking change potential for somebody changing the padding values with the theme.overrides. Should we care?

@oliviertassinari oliviertassinari changed the title Add disableGutters prop to Container component [Container] Add disableGutters prop May 27, 2019
@oliviertassinari oliviertassinari added component: Container The React component new feature New feature or request labels May 27, 2019
@divyanshutomar
Copy link
Contributor Author

Yup, it will be a breaking change for such use cases. Assuming that we can't assess the impact of this, we can probably hold this up until the next major release. It will be great to have other's opinions about this.

@oliviertassinari
Copy link
Member

@divyanshutomar The next major release won't happen before a year.

@oliviertassinari
Copy link
Member

@divyanshutomar Ok, we can wait for v5. Do you think that you could split the pull request in 2? The CSS update is great to take on its own :).

@divyanshutomar
Copy link
Contributor Author

@oliviertassinari Sure, I will send a PR that uses gutters class to add padding to the existing Container component. And, update this one with the disableGutters props changes.

@oliviertassinari oliviertassinari added the on hold There is a blocker, we need to wait label May 29, 2019
@oliviertassinari oliviertassinari force-pushed the container/disable-gutters-support branch from c1649b4 to 29ee2bd Compare July 21, 2019 21:46
@oliviertassinari
Copy link
Member

Rebased.

@oliviertassinari oliviertassinari removed the new feature New feature or request label Jul 21, 2019
@oliviertassinari oliviertassinari added new feature New feature or request and removed on hold There is a blocker, we need to wait breaking change labels Dec 7, 2019
@oliviertassinari oliviertassinari removed this from the v5 milestone Dec 7, 2019
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

I have updated the approach to be nonbreaking, I start to even think that it's a better approach. I think that we can move forward with it.

@oliviertassinari oliviertassinari merged commit acaae6e into mui:master Dec 8, 2019
@oliviertassinari
Copy link
Member

@divyanshutomar I'm sorry it took so long. Thanks for the effort :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Container The React component new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing disableGutters prop on Container
4 participants