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.

Sign up for GitHub

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

Jump to bottom

[ScopedCssBaseline] Allow css to be only applied on children #19669

Merged
merged 4 commits into from
Feb 14, 2020
Merged

[ScopedCssBaseline] Allow css to be only applied on children #19669

merged 4 commits into from
Feb 14, 2020

Conversation

TomPradat
Copy link
Contributor

@TomPradat TomPradat commented Feb 12, 2020 •
edited by oliviertassinari
Loading

Fixes #19657
Fixes #17244

@mui-pr-bot
Copy link

mui-pr-bot commented Feb 12, 2020 •
edited
Loading

Details of bundle changes.

Comparing: 9f54e27...28f3506

bundle Size Change Size Gzip Change Gzip
ScopedCssBaseline ▲ +63.2 kB (+Infinity% ) 63.2 kB ▲ +19.9 kB (+Infinity% ) 19.9 kB
CssBaseline ▲ +4.48 kB (+7.75% ) 62.3 kB ▲ +1.47 kB (+8.10% ) 19.6 kB
@material-ui/core ▲ +82 B (+0.02% ) 362 kB ▲ +49 B (+0.05% ) 98.9 kB
docs.main ▲ +80 B (+0.01% ) 604 kB ▲ +21 B (+0.01% ) 196 kB
@material-ui/core[umd] ▲ +71 B (+0.02% ) 318 kB ▲ +39 B (+0.04% ) 92 kB
TablePagination ▲ +1 B (0.00% ) 144 kB ▼ -8 B (-0.02% ) 42 kB
TextField ▼ -1 B (-0.00% ) 126 kB ▲ +4 B (+0.01% ) 36.6 kB
@material-ui/lab ▼ -1 B (-0.00% ) 193 kB ▼ -2 B (-0.00% ) 57.2 kB
SpeedDialIcon -- 64.9 kB ▼ -61 B (-0.30% ) 20.3 kB
TableSortLabel -- 77.8 kB ▲ +42 B (+0.17% ) 24.5 kB
Backdrop -- 68.2 kB ▲ +10 B (+0.05% ) 21.1 kB
Collapse -- 68.4 kB ▲ +10 B (+0.05% ) 21.2 kB
FormLabel -- 63.8 kB ▲ +10 B (+0.05% ) 19.8 kB
Rating -- 70.8 kB ▲ +10 B (+0.04% ) 22.8 kB
@material-ui/styles -- 51.4 kB ▲ +9 B (+0.06% ) 15.4 kB
FormControl -- 64.8 kB ▲ +9 B (+0.04% ) 20.2 kB
Typography -- 64 kB ▲ +9 B (+0.04% ) 20 kB
MenuList -- 66.4 kB ▲ +8 B (+0.04% ) 20.8 kB
SvgIcon -- 63.4 kB ▲ +8 B (+0.04% ) 19.8 kB
Checkbox -- 83.3 kB ▲ +7 B (+0.03% ) 26.3 kB
GridList -- 62.8 kB ▲ +7 B (+0.04% ) 19.7 kB
StepIcon -- 65 kB ▲ +7 B (+0.03% ) 20.3 kB
List -- 62.7 kB ▲ +6 B (+0.03% ) 19.6 kB
Paper -- 62.7 kB ▲ +6 B (+0.03% ) 19.6 kB
RadioGroup -- 64.8 kB ▲ +6 B (+0.03% ) 20.1 kB
Alert -- 83.8 kB ▲ +5 B (+0.02% ) 26.3 kB
CardHeader -- 65.4 kB ▼ -5 B (-0.02% ) 20.6 kB
ExpansionPanel -- 72.7 kB ▼ -5 B (-0.02% ) 22.7 kB
InputBase -- 70.9 kB ▲ +5 B (+0.02% ) 22.3 kB
ListItem -- 77.5 kB ▲ +5 B (+0.02% ) 24.2 kB
Switch -- 82.5 kB ▲ +5 B (+0.02% ) 26 kB
ToggleButtonGroup -- 63.5 kB ▼ -5 B (-0.02% ) 20 kB
Badge -- 65.7 kB ▲ +4 B (+0.02% ) 20.4 kB
BottomNavigation -- 62.7 kB ▲ +4 B (+0.02% ) 19.7 kB
ButtonBase -- 74.4 kB ▲ +4 B (+0.02% ) 23.3 kB
Chip -- 83 kB ▼ -4 B (-0.02% ) 25.4 kB
Container -- 63.5 kB ▼ -4 B (-0.02% ) 19.9 kB
FilledInput -- 73.9 kB ▲ +4 B (+0.02% ) 23 kB
Input -- 72.8 kB ▲ +4 B (+0.02% ) 22.8 kB
MenuItem -- 78.6 kB ▲ +4 B (+0.02% ) 24.5 kB
OutlinedInput -- 74.9 kB ▲ +4 B (+0.02% ) 23.3 kB
Popover -- 83.5 kB ▲ +4 B (+0.02% ) 25.8 kB
Tab -- 76.7 kB ▲ +4 B (+0.02% ) 24.3 kB
Tooltip -- 103 kB ▲ +4 B (+0.01% ) 32.4 kB
Autocomplete -- 132 kB ▲ +3 B (+0.01% ) 41.4 kB
Avatar -- 65.6 kB ▼ -3 B (-0.01% ) 20.7 kB
AvatarGroup -- 62.6 kB ▲ +3 B (+0.02% ) 19.7 kB
DialogActions -- 62.4 kB ▼ -3 B (-0.02% ) 19.6 kB
InputAdornment -- 65.4 kB ▼ -3 B (-0.01% ) 20.6 kB
LinearProgress -- 65.7 kB ▲ +3 B (+0.01% ) 20.5 kB
Pagination -- 85.5 kB ▼ -3 B (-0.01% ) 26.3 kB
PaginationItem -- 81.2 kB ▼ -3 B (-0.01% ) 25 kB
Select -- 117 kB ▲ +3 B (+0.01% ) 34.6 kB
Snackbar -- 75.7 kB ▲ +3 B (+0.01% ) 23.7 kB
SnackbarContent -- 63.9 kB ▲ +3 B (+0.01% ) 20.1 kB
SpeedDialAction -- 119 kB ▲ +3 B (+0.01% ) 37.6 kB
StepButton -- 82.7 kB ▼ -3 B (-0.01% ) 26.2 kB
Table -- 62.9 kB ▲ +3 B (+0.02% ) 19.7 kB
TableContainer -- 62.3 kB ▲ +3 B (+0.02% ) 19.5 kB
Tabs -- 85.8 kB ▲ +3 B (+0.01% ) 27.2 kB
AlertTitle -- 64.5 kB ▲ +2 B (+0.01% ) 20.3 kB
Breadcrumbs -- 68.1 kB ▼ -2 B (-0.01% ) 21.4 kB
CardActions -- 62.4 kB ▼ -2 B (-0.01% ) 19.6 kB
CardContent -- 62.3 kB ▲ +2 B (+0.01% ) 19.5 kB
Dialog -- 83.4 kB ▼ -2 B (-0.01% ) 26 kB
DialogContentText -- 64.4 kB ▼ -2 B (-0.01% ) 20.2 kB
DialogTitle -- 64.6 kB ▲ +2 B (+0.01% ) 20.3 kB
ExpansionPanelActions -- 62.4 kB ▼ -2 B (-0.01% ) 19.6 kB
FormControlLabel -- 65.9 kB ▼ -2 B (-0.01% ) 20.6 kB
FormGroup -- 62.3 kB ▲ +2 B (+0.01% ) 19.6 kB
Icon -- 63.1 kB ▲ +2 B (+0.01% ) 19.8 kB
ListItemText -- 65.3 kB ▼ -2 B (-0.01% ) 20.5 kB
MobileStepper -- 68.2 kB ▼ -2 B (-0.01% ) 21.4 kB
NativeSelect -- 77.2 kB ▲ +2 B (+0.01% ) 24.3 kB
Radio -- 84.4 kB ▲ +2 B (+0.01% ) 26.6 kB
RootRef -- 4.24 kB ▼ -2 B (-0.12% ) 1.64 kB
Step -- 63 kB ▼ -2 B (-0.01% ) 19.8 kB
StepContent -- 69.5 kB ▼ -2 B (-0.01% ) 21.8 kB
StepLabel -- 69 kB ▲ +2 B (+0.01% ) 21.7 kB
ToggleButton -- 76.5 kB ▲ +2 B (+0.01% ) 24.2 kB
@material-ui/system -- 16.5 kB ▲ +1 B (+0.02% ) 4.32 kB
Box -- 72.2 kB ▼ -1 B (-0.00% ) 21.9 kB
Button -- 80.1 kB ▼ -1 B (-0.00% ) 24.5 kB
ButtonGroup -- 83.6 kB ▲ +1 B (0.00% ) 25.6 kB
Card -- 63.2 kB ▲ +1 B (+0.01% ) 19.8 kB
CardActionArea -- 75.5 kB ▲ +1 B (0.00% ) 23.8 kB
CardMedia -- 62.7 kB ▲ +1 B (+0.01% ) 19.7 kB
CircularProgress -- 64.4 kB ▲ +1 B (0.00% ) 20.3 kB
DialogContent -- 62.6 kB ▲ +1 B (+0.01% ) 19.6 kB
Divider -- 63 kB ▼ -1 B (-0.01% ) 19.8 kB
Drawer -- 85.2 kB ▼ -1 B (-0.00% ) 25.9 kB
ExpansionPanelDetails -- 62.3 kB ▲ +1 B (+0.01% ) 19.5 kB
ExpansionPanelSummary -- 78.5 kB ▲ +1 B (0.00% ) 24.8 kB
Fab -- 77.2 kB ▼ -1 B (-0.00% ) 24.1 kB
FormHelperText -- 63.7 kB ▲ +1 B (0.00% ) 20 kB
Hidden -- 66.3 kB ▼ -1 B (-0.00% ) 20.8 kB
IconButton -- 76.5 kB ▼ -1 B (-0.00% ) 23.9 kB
InputLabel -- 65.7 kB ▲ +1 B (0.00% ) 20.5 kB
Link -- 67 kB ▼ -1 B (-0.00% ) 21.1 kB
ListItemAvatar -- 62.4 kB ▲ +1 B (+0.01% ) 19.5 kB
ListItemIcon -- 62.5 kB ▲ +1 B (+0.01% ) 19.6 kB
ListSubheader -- 63.1 kB ▲ +1 B (+0.01% ) 19.8 kB
Menu -- 89.1 kB ▲ +1 B (0.00% ) 27.4 kB
Popper -- 28.8 kB ▲ +1 B (+0.01% ) 10.3 kB
Skeleton -- 63.3 kB ▲ +1 B (0.00% ) 20 kB
Slider -- 77 kB ▲ +1 B (0.00% ) 24.2 kB
SpeedDial -- 86.6 kB ▲ +1 B (0.00% ) 27.3 kB
StepConnector -- 63.1 kB ▲ +1 B (+0.01% ) 19.9 kB
Stepper -- 65.2 kB ▲ +1 B (0.00% ) 20.6 kB
TableBody -- 62.4 kB ▼ -1 B (-0.01% ) 19.5 kB
TableCell -- 64.4 kB ▲ +1 B (0.00% ) 20.3 kB
TableHead -- 62.4 kB ▼ -1 B (-0.01% ) 19.5 kB
TableRow -- 62.8 kB ▲ +1 B (+0.01% ) 19.7 kB
Toolbar -- 62.7 kB ▲ +1 B (+0.01% ) 19.7 kB
TreeItem -- 74.3 kB ▼ -1 B (-0.00% ) 23.5 kB
TreeView -- 67 kB ▲ +1 B (0.00% ) 21.1 kB
AppBar -- 64.4 kB -- 20.2 kB
BottomNavigationAction -- 75.9 kB -- 24 kB
ClickAwayListener -- 3.91 kB -- 1.55 kB
colorManipulator -- 3.88 kB -- 1.52 kB
docs.landing -- 56.4 kB -- 14.5 kB
Fade -- 23.6 kB -- 8.01 kB
Grid -- 65.4 kB -- 20.5 kB
GridListTile -- 64.1 kB -- 20.1 kB
GridListTileBar -- 63.6 kB -- 19.9 kB
Grow -- 24.2 kB -- 8.22 kB
ListItemSecondaryAction -- 62.3 kB -- 19.5 kB
Modal -- 14.5 kB -- 5.04 kB
NoSsr -- 2.19 kB -- 1.04 kB
Portal -- 2.92 kB -- 1.3 kB
Slide -- 25.7 kB -- 8.74 kB
styles/createMuiTheme -- 16.6 kB -- 5.85 kB
SwipeableDrawer -- 92.6 kB -- 28.9 kB
TableFooter -- 62.4 kB -- 19.5 kB
TextareaAutosize -- 5.12 kB -- 2.14 kB
useAutocomplete -- 14.7 kB -- 5.31 kB
useMediaQuery -- 2.58 kB -- 1.06 kB
Zoom -- 23.6 kB -- 8.12 kB

Generated by 🚫 dangerJS against 28f3506

@TomPradat
Copy link
Contributor Author

I'm not sure why the test:umd fails, seems like a timeout. It works on my local computer

@TomPradat TomPradat changed the title WIP : [CssBaseline] Allow css to be only applied on children [CssBaseline] Allow css to be only applied on children Feb 12, 2020
@oliviertassinari oliviertassinari added the component: CssBaseline The React component label Feb 12, 2020
oliviertassinari
oliviertassinari previously requested changes Feb 12, 2020
View reviewed changes
Copy link
Member

@oliviertassinari oliviertassinari left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It's a great start, thanks for working on it.

However, I think that we will need to change the strategy. The scopeToChildren prop fundamentally changes the structure of the component. This turns into a couple of problems: forward ref that won't always work, prop spreading conflict with the exact prop types, style hook that won't be possible with styled components migration.

In this case, I think that we should go with a new component. What about ScopedCssBaseline?

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 12, 2020 •
edited
Loading

Note 1. We might want to use withStyles over the hook to fix #17244, now is a good opportunity.
Note 2. I don't understand why we use font weight: 'bolder' over the value from the theme
Note 3. For v5, I have started to gather ideas for the reset: https://trello.com/c/U5K9pVIC/2001-improve-css-reset

@oliviertassinari oliviertassinari added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI new feature New feature or request labels Feb 12, 2020
@oliviertassinari oliviertassinari force-pushed the feature/cssbaseline-isolation branch from 405fffa to 889e753 Compare February 13, 2020 13:46
@oliviertassinari oliviertassinari removed the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Feb 13, 2020
@oliviertassinari oliviertassinari force-pushed the feature/cssbaseline-isolation branch from 889e753 to 968ef44 Compare February 13, 2020 13:54
@joshwooding
Copy link
Member

I think the title or the description should mention that a new component has been added.

@oliviertassinari oliviertassinari changed the title [CssBaseline] Allow css to be only applied on children [ScopedCssBaseline] Allow css to be only applied on children Feb 14, 2020
@oliviertassinari oliviertassinari merged commit c06a88d into mui:master Feb 14, 2020
@oliviertassinari
Copy link
Member

@joshwooding thanks for the tip

This was referenced Mar 10, 2020
Merged
Merged
Merged
Closed
Closed
Closed
Open
@snyk-bot snyk-bot mentioned this pull request Mar 21, 2020
Merged
@snyk-bot snyk-bot mentioned this pull request Mar 22, 2020
Open
This was referenced Apr 16, 2020
Merged
Closed
@snyk-bot snyk-bot mentioned this pull request May 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oliviertassinari oliviertassinari oliviertassinari left review comments

Assignees
No one assigned
Labels
component: CssBaseline The React component new feature New feature or request
Projects
None yet
Milestone
No milestone
4 participants
@TomPradat @mui-pr-bot @oliviertassinari @joshwooding