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

[TextField] Avoid outline label CSS leak #19937

Merged
merged 4 commits into from
Mar 4, 2020

Conversation

ivoiv
Copy link
Contributor

@ivoiv ivoiv commented Mar 2, 2020

Currently the styles of the label in NotchedOutline of the TextField is set by the '& span' selector in the legendLabelled class. It basically adds padding.

When a label prop comes in, it's wrapped in a <span> before it's inserted.
This means that if you also send a <span> element as the label, you get a <span><span>My Label</span></span> for the label.

Because of the '& span' selector, both elements inherit the style, meaning you get double padding and thus extra empty space around the notch.

By changing the selector to a class, the padding will no longer be applied to other span elements sent to the label component.

When sending labels other than span, like div, the label span does not inherit the width of the div meaning it's only default padding(5px on each side) wide.
By setting the span display to to a 'inline-block'enables the label to expand to the width of other, non-string components sent to the label.

Given code
<TextField variant="outlined" label={<span>My Label</span>}/>

Current behaviour
image
image

New behaviour
image
This now also works with
<TextField variant="outlined" label={<div>My Label</div>}/>

@ivoiv ivoiv changed the title Replaced global span style with specific class Replaced global span selector in NotchedOutline with specific class Mar 2, 2020
@ivoiv
Copy link
Contributor Author

ivoiv commented Mar 2, 2020

ci/circleci: test_static seems to fail on code that was there from before, the dangerouslySetInnerHTML

What to do?

@mui-pr-bot
Copy link

mui-pr-bot commented Mar 2, 2020

Details of bundle changes.

Comparing: 33a7c26...bd72783

bundle Size Change Size Gzip Change Gzip
TablePagination ▲ +25 B (+0.02% ) 145 kB ▲ +13 B (+0.03% ) 42.7 kB
@material-ui/core[umd] ▲ +25 B (+0.01% ) 318 kB ▲ +10 B (+0.01% ) 92.1 kB
@material-ui/core ▲ +25 B (+0.01% ) 360 kB ▲ +7 B (+0.01% ) 98.8 kB
OutlinedInput ▲ +25 B (+0.03% ) 77.9 kB ▲ +7 B (+0.03% ) 24.3 kB
Select ▲ +25 B (+0.02% ) 119 kB ▲ +7 B (+0.02% ) 35.3 kB
TextField ▲ +25 B (+0.02% ) 127 kB ▲ +5 B (+0.01% ) 37.4 kB
@material-ui/lab -- 198 kB -- 58.8 kB
@material-ui/styles -- 51.4 kB -- 15.4 kB
@material-ui/system -- 16.5 kB -- 4.31 kB
Alert -- 86.6 kB -- 27.3 kB
AlertTitle -- 67.5 kB -- 21.2 kB
AppBar -- 67.4 kB -- 21.1 kB
Autocomplete -- 134 kB -- 42.1 kB
Avatar -- 68.5 kB -- 21.7 kB
AvatarGroup -- 65.7 kB -- 20.6 kB
Backdrop -- 71.2 kB -- 22 kB
Badge -- 68.7 kB -- 21.3 kB
BottomNavigation -- 65.7 kB -- 20.6 kB
BottomNavigationAction -- 78.8 kB -- 24.9 kB
Box -- 72.3 kB -- 21.8 kB
Breadcrumbs -- 83.6 kB -- 26.5 kB
Button -- 83 kB -- 25.4 kB
ButtonBase -- 77.3 kB -- 24.2 kB
ButtonGroup -- 86.5 kB -- 26.6 kB
Card -- 66.2 kB -- 20.7 kB
CardActionArea -- 78.4 kB -- 24.8 kB
CardActions -- 65.4 kB -- 20.5 kB
CardContent -- 65.3 kB -- 20.4 kB
CardHeader -- 68.4 kB -- 21.5 kB
CardMedia -- 65.7 kB -- 20.6 kB
Checkbox -- 85.4 kB -- 27 kB
Chip -- 85.8 kB -- 26.3 kB
CircularProgress -- 67.4 kB -- 21.2 kB
ClickAwayListener -- 3.84 kB -- 1.54 kB
Collapse -- 71.3 kB -- 22.1 kB
colorManipulator -- 3.88 kB -- 1.52 kB
Container -- 66.5 kB -- 20.8 kB
CssBaseline -- 65.3 kB -- 20.5 kB
Dialog -- 86.2 kB -- 26.9 kB
DialogActions -- 65.4 kB -- 20.5 kB
DialogContent -- 65.6 kB -- 20.5 kB
DialogContentText -- 67.4 kB -- 21.1 kB
DialogTitle -- 67.6 kB -- 21.2 kB
Divider -- 66 kB -- 20.7 kB
docs.landing -- 56.6 kB -- 15.6 kB
docs.main -- 602 kB -- 195 kB
Drawer -- 87.9 kB -- 26.8 kB
ExpansionPanel -- 74.9 kB -- 23.5 kB
ExpansionPanelActions -- 65.4 kB -- 20.5 kB
ExpansionPanelDetails -- 65.3 kB -- 20.4 kB
ExpansionPanelSummary -- 81.4 kB -- 25.7 kB
Fab -- 80.1 kB -- 24.9 kB
Fade -- 27.1 kB -- 9.06 kB
FilledInput -- 76.9 kB -- 23.9 kB
FormControl -- 67.7 kB -- 21.1 kB
FormControlLabel -- 68.8 kB -- 21.6 kB
FormGroup -- 65.4 kB -- 20.5 kB
FormHelperText -- 66.7 kB -- 20.7 kB
FormLabel -- 66.8 kB -- 20.7 kB
Grid -- 68.4 kB -- 21.4 kB
GridList -- 65.8 kB -- 20.6 kB
GridListTile -- 67.1 kB -- 21 kB
GridListTileBar -- 66.6 kB -- 20.8 kB
Grow -- 27.7 kB -- 9.26 kB
Hidden -- 69.3 kB -- 21.7 kB
Icon -- 66.1 kB -- 20.7 kB
IconButton -- 79.4 kB -- 24.8 kB
Input -- 75.9 kB -- 23.7 kB
InputAdornment -- 68.4 kB -- 21.6 kB
InputBase -- 74 kB -- 23.2 kB
InputLabel -- 68.6 kB -- 21.2 kB
LinearProgress -- 68.7 kB -- 21.2 kB
Link -- 69.9 kB -- 22.1 kB
List -- 65.7 kB -- 20.5 kB
ListItem -- 80.3 kB -- 25.1 kB
ListItemAvatar -- 65.4 kB -- 20.5 kB
ListItemIcon -- 65.5 kB -- 20.5 kB
ListItemSecondaryAction -- 65.4 kB -- 20.5 kB
ListItemText -- 68.3 kB -- 21.5 kB
ListSubheader -- 66.1 kB -- 20.8 kB
Menu -- 91.8 kB -- 28.3 kB
MenuItem -- 81.4 kB -- 25.4 kB
MenuList -- 69.3 kB -- 21.7 kB
MobileStepper -- 71.2 kB -- 22.3 kB
Modal -- 14.3 kB -- 5.04 kB
NativeSelect -- 80.1 kB -- 25.3 kB
NoSsr -- 2.17 kB -- 1.03 kB
Pagination -- 87.6 kB -- 27 kB
PaginationItem -- 84 kB -- 25.9 kB
Paper -- 65.7 kB -- 20.5 kB
Popover -- 86.2 kB -- 26.7 kB
Popper -- 28.8 kB -- 10.3 kB
Portal -- 2.87 kB -- 1.29 kB
Radio -- 86.4 kB -- 27.3 kB
RadioGroup -- 67.1 kB -- 20.9 kB
Rating -- 73.7 kB -- 23.7 kB
RootRef -- 4.21 kB -- 1.64 kB
ScopedCssBaseline -- 66.2 kB -- 20.7 kB
Skeleton -- 66.3 kB -- 20.9 kB
Slide -- 29.1 kB -- 9.78 kB
Slider -- 79.1 kB -- 25.2 kB
Snackbar -- 78.6 kB -- 24.5 kB
SnackbarContent -- 66.9 kB -- 21 kB
SpeedDial -- 89.4 kB -- 28.3 kB
SpeedDialAction -- 121 kB -- 38.4 kB
SpeedDialIcon -- 67.9 kB -- 21.3 kB
Step -- 66 kB -- 20.7 kB
StepButton -- 85.5 kB -- 27 kB
StepConnector -- 66.1 kB -- 20.8 kB
StepContent -- 72.5 kB -- 22.6 kB
StepIcon -- 67.9 kB -- 21.2 kB
StepLabel -- 71.9 kB -- 22.3 kB
Stepper -- 68.2 kB -- 21.5 kB
styles/createMuiTheme -- 20.9 kB -- 7.27 kB
SvgIcon -- 66.4 kB -- 20.7 kB
SwipeableDrawer -- 95.3 kB -- 29.9 kB
Switch -- 84.6 kB -- 26.6 kB
Tab -- 79.6 kB -- 25.3 kB
Table -- 65.9 kB -- 20.6 kB
TableBody -- 65.4 kB -- 20.5 kB
TableCell -- 67.4 kB -- 21.2 kB
TableContainer -- 65.3 kB -- 20.4 kB
TableFooter -- 65.5 kB -- 20.5 kB
TableHead -- 65.4 kB -- 20.5 kB
TableRow -- 65.8 kB -- 20.6 kB
TableSortLabel -- 80.6 kB -- 25.5 kB
Tabs -- 88.6 kB -- 28.3 kB
TextareaAutosize -- 5.19 kB -- 2.16 kB
ToggleButton -- 79.4 kB -- 25.2 kB
ToggleButtonGroup -- 66.5 kB -- 20.9 kB
Toolbar -- 65.7 kB -- 20.6 kB
Tooltip -- 105 kB -- 33.1 kB
TreeItem -- 78.5 kB -- 24.8 kB
TreeView -- 71.5 kB -- 22.4 kB
Typography -- 67 kB -- 20.9 kB
useAutocomplete -- 14.5 kB -- 5.25 kB
useMediaQuery -- 2.56 kB -- 1.06 kB
Zoom -- 27.1 kB -- 9.18 kB

Generated by 🚫 dangerJS against bd72783

@ivoiv ivoiv changed the title Replaced global span selector in NotchedOutline with specific class [NotchedOutline]Replaced global span selector a with specific class Mar 2, 2020
@ivoiv ivoiv changed the title [NotchedOutline]Replaced global span selector a with specific class [NotchedOutline] Replaced global span selector for label with a specific class. Added support for non-string labels. Mar 2, 2020
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.

@oliviertassinari Why do we actually need a separate span here?

packages/material-ui/src/OutlinedInput/NotchedOutline.js Outdated Show resolved Hide resolved
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.

Would the following be enough?

-& span
+& > span

@oliviertassinari
Copy link
Member

@eps1lon I can't remember, we would need to look at the history.

@oliviertassinari oliviertassinari added the component: text field This is the name of the generic UI component, not the React module! label Mar 2, 2020
@ivoiv
Copy link
Contributor Author

ivoiv commented Mar 2, 2020

@oliviertassinari
That's a great suggestion, yes!
Should have thought of it.

Changed the PR.

@ivoiv ivoiv changed the title [NotchedOutline] Replaced global span selector for label with a specific class. Added support for non-string labels. [NotchedOutline] Increased selector specificity for label element. Added support for non-string labels. Mar 2, 2020
@ivoiv ivoiv requested a review from eps1lon March 2, 2020 19:06
@eps1lon
Copy link
Member

eps1lon commented Mar 2, 2020

& > span

I would still prefer a simple classname while we're at it. We can have proper documentation for that. That isn't the case for custom selectors.

@oliviertassinari
Copy link
Member

Considering the component is private, I wonder about the advantages between the two approaches.

@oliviertassinari oliviertassinari changed the title [NotchedOutline] Increased selector specificity for label element. Added support for non-string labels. [TextField] Avoid outline label CSS leak Mar 3, 2020
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Mar 3, 2020
@oliviertassinari
Copy link
Member

@ivoiv Thanks for fixing this edge case :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: text field 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.

4 participants