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

[Alert] Introduce new component #18702

Merged
merged 12 commits into from
Jan 3, 2020
Merged

[Alert] Introduce new component #18702

merged 12 commits into from
Jan 3, 2020

Conversation

dimitropoulos
Copy link
Member

@dimitropoulos dimitropoulos commented Dec 6, 2019 •
edited by oliviertassinari
Loading

@dimitropoulos dimitropoulos mentioned this pull request Dec 6, 2019
Closed
1 task
@mui-pr-bot
Copy link

mui-pr-bot commented Dec 6, 2019 •
edited
Loading

@material-ui/core: parsed: 0.00% 😍, gzip: +0.53%
@material-ui/lab: parsed: +2.99% , gzip: +2.25%

Details of bundle changes.

Comparing: a07f180...5795edc

bundle Size Change Size Gzip Change Gzip
Alert ▲ +83.5 kB (+Infinity% ) 83.5 kB ▲ +26.2 kB (+Infinity% ) 26.2 kB
AlertTitle ▲ +63.9 kB (+Infinity% ) 63.9 kB ▲ +20.2 kB (+Infinity% ) 20.2 kB
@material-ui/lab ▲ +5.3 kB (+2.99% ) 182 kB ▲ +1.2 kB (+2.25% ) 54.6 kB
docs.main ▲ +256 B (+0.04% ) 614 kB ▲ +55 B (+0.03% ) 196 kB
Autocomplete ▲ +70 B (+0.05% ) 129 kB ▲ +191 B (+0.47% ) 40.6 kB
TablePagination ▲ +7 B (0.00% ) 141 kB ▲ +8 B (+0.02% ) 41.3 kB
@material-ui/core ▲ +6 B (0.00% ) 358 kB ▲ +521 B (+0.53% ) 98.2 kB
TextField ▲ +6 B (0.00% ) 123 kB -- 36.1 kB
Menu ▲ +3 B (0.00% ) 88.2 kB ▲ +3 B (+0.01% ) 27.3 kB
Grow ▲ +3 B (+0.01% ) 23.9 kB ▲ +2 B (+0.02% ) 8.21 kB
IconButton ▼ -3 B (-0.00% ) 75.9 kB ▲ +1 B (0.00% ) 23.7 kB
SpeedDialAction ▲ +2 B (0.00% ) 117 kB ▲ +51 B (+0.14% ) 37.1 kB
Snackbar ▲ +2 B (0.00% ) 74.9 kB ▲ +30 B (+0.13% ) 23.5 kB
Rating ▼ -2 B (-0.00% ) 70 kB ▼ -22 B (-0.10% ) 22.5 kB
SpeedDialIcon ▼ -2 B (-0.00% ) 64.3 kB ▼ -14 B (-0.07% ) 20.2 kB
Drawer ▲ +2 B (0.00% ) 84.2 kB ▲ +6 B (+0.02% ) 25.7 kB
Typography ▲ +2 B (0.00% ) 63.4 kB ▲ +3 B (+0.02% ) 19.9 kB
Popover ▲ +2 B (0.00% ) 82.5 kB -- 25.7 kB
Select ▲ +2 B (0.00% ) 114 kB -- 34 kB
Tooltip ▲ +2 B (0.00% ) 101 kB -- 31.9 kB
StepLabel ▲ +1 B (0.00% ) 68.3 kB ▲ +364 B (+1.72% ) 21.6 kB
Button ▲ +1 B (0.00% ) 79.4 kB ▲ +3 B (+0.01% ) 24.4 kB
Fab ▲ +1 B (0.00% ) 76.5 kB ▲ +3 B (+0.01% ) 23.9 kB
FormControl ▲ +1 B (0.00% ) 64.1 kB ▲ +3 B (+0.01% ) 20 kB
FormLabel ▲ +1 B (0.00% ) 63.2 kB ▲ +3 B (+0.02% ) 19.7 kB
Paper ▲ +1 B (0.00% ) 62.1 kB ▲ +3 B (+0.02% ) 19.4 kB
Backdrop ▲ +1 B (0.00% ) 67.5 kB ▲ +2 B (+0.01% ) 20.9 kB
Chip ▲ +1 B (0.00% ) 82.3 kB ▲ +2 B (+0.01% ) 25.3 kB
InputBase ▲ +1 B (0.00% ) 70.3 kB ▲ +2 B (+0.01% ) 22.1 kB
InputLabel ▲ +1 B (0.00% ) 65.1 kB ▲ +2 B (+0.01% ) 20.1 kB
OutlinedInput ▲ +1 B (0.00% ) 73.7 kB ▲ +2 B (+0.01% ) 23 kB
SvgIcon ▲ +1 B (0.00% ) 62.8 kB ▲ +2 B (+0.01% ) 19.7 kB
ButtonBase ▲ +1 B (0.00% ) 73.7 kB ▲ +1 B (0.00% ) 23.2 kB
FilledInput ▲ +1 B (0.00% ) 73.3 kB ▲ +1 B (0.00% ) 22.8 kB
ListItem ▲ +1 B (0.00% ) 76.9 kB ▲ +1 B (0.00% ) 24.1 kB
MenuItem ▲ +1 B (0.00% ) 77.9 kB ▲ +1 B (0.00% ) 24.4 kB
MenuList ▲ +1 B (0.00% ) 65.7 kB ▲ +1 B (0.00% ) 20.6 kB
StepIcon ▲ +1 B (0.00% ) 64.4 kB -- 20.1 kB
CardHeader -- 64.8 kB ▼ -35 B (-0.17% ) 20.4 kB
Stepper -- 64.6 kB ▼ -28 B (-0.14% ) 20.4 kB
SpeedDial -- 85.8 kB ▲ +11 B (+0.04% ) 27.1 kB
Box -- 70.5 kB ▲ +9 B (+0.04% ) 21.5 kB
Slider -- 75.3 kB ▼ -9 B (-0.04% ) 23.9 kB
ExpansionPanelActions -- 61.8 kB ▲ +7 B (+0.04% ) 19.4 kB
StepButton -- 82 kB ▲ +7 B (+0.03% ) 26 kB
StepConnector -- 62.5 kB ▼ -7 B (-0.04% ) 19.7 kB
ToggleButtonGroup -- 63 kB ▲ +7 B (+0.04% ) 19.9 kB
Icon -- 62.5 kB ▲ +6 B (+0.03% ) 19.7 kB
TableRow -- 62.3 kB ▼ -6 B (-0.03% ) 19.6 kB
@material-ui/system -- 14.5 kB ▲ +5 B (+0.12% ) 4.04 kB
MobileStepper -- 67.6 kB ▲ +5 B (+0.02% ) 21.3 kB
Skeleton -- 62.7 kB ▲ +5 B (+0.03% ) 19.9 kB
TreeView -- 66.1 kB ▲ +5 B (+0.02% ) 20.8 kB
Dialog -- 82.5 kB ▲ +4 B (+0.02% ) 25.9 kB
DialogContent -- 62 kB ▲ +4 B (+0.02% ) 19.5 kB
DialogContentText -- 63.8 kB ▼ -4 B (-0.02% ) 20.1 kB
Divider -- 62.3 kB ▲ +4 B (+0.02% ) 19.7 kB
GridList -- 62.2 kB ▲ +4 B (+0.02% ) 19.6 kB
ListItemSecondaryAction -- 61.8 kB ▲ +4 B (+0.02% ) 19.4 kB
Switch -- 80.9 kB ▼ -4 B (-0.02% ) 25.5 kB
TableFooter -- 61.9 kB ▼ -4 B (-0.02% ) 19.4 kB
TableHead -- 61.9 kB ▼ -4 B (-0.02% ) 19.4 kB
CardActions -- 61.8 kB ▲ +3 B (+0.02% ) 19.4 kB
Checkbox -- 81.7 kB ▼ -3 B (-0.01% ) 25.9 kB
CssBaseline -- 57.3 kB ▲ +3 B (+0.02% ) 18 kB
GridListTileBar -- 63 kB ▼ -3 B (-0.02% ) 19.8 kB
Hidden -- 65.7 kB ▲ +3 B (+0.01% ) 20.7 kB
Input -- 72.2 kB ▲ +3 B (+0.01% ) 22.6 kB
TableSortLabel -- 77.1 kB ▼ -3 B (-0.01% ) 24.3 kB
TreeItem -- 73.5 kB ▲ +3 B (+0.01% ) 23.3 kB
@material-ui/styles -- 50.8 kB ▼ -2 B (-0.01% ) 15.3 kB
Avatar -- 65 kB ▲ +2 B (+0.01% ) 20.6 kB
BottomNavigationAction -- 75.2 kB ▲ +2 B (+0.01% ) 23.8 kB
ButtonGroup -- 82.9 kB ▼ -2 B (-0.01% ) 25.5 kB
CardActionArea -- 74.8 kB ▲ +2 B (+0.01% ) 23.6 kB
CardMedia -- 62.1 kB ▲ +2 B (+0.01% ) 19.6 kB
Collapse -- 67.6 kB ▲ +2 B (+0.01% ) 21 kB
DialogActions -- 61.9 kB ▲ +2 B (+0.01% ) 19.5 kB
DialogTitle -- 64 kB ▼ -2 B (-0.01% ) 20.1 kB
ExpansionPanel -- 71.1 kB ▲ +2 B (+0.01% ) 22.3 kB
ExpansionPanelSummary -- 77.8 kB ▼ -2 B (-0.01% ) 24.6 kB
Grid -- 64.9 kB ▲ +2 B (+0.01% ) 20.4 kB
List -- 62.1 kB ▲ +2 B (+0.01% ) 19.4 kB
ListSubheader -- 62.5 kB ▲ +2 B (+0.01% ) 19.7 kB
Modal -- 14.3 kB ▲ +2 B (+0.04% ) 5.01 kB
NoSsr -- 2.19 kB ▼ -2 B (-0.19% ) 1.03 kB
RadioGroup -- 63.2 kB ▲ +2 B (+0.01% ) 19.9 kB
TableContainer -- 61.7 kB ▼ -2 B (-0.01% ) 19.4 kB
Toolbar -- 62.1 kB ▲ +2 B (+0.01% ) 19.6 kB
Zoom -- 23.4 kB ▲ +2 B (+0.02% ) 8.11 kB
AvatarGroup -- 62 kB ▲ +1 B (+0.01% ) 19.5 kB
Breadcrumbs -- 67.7 kB ▼ -1 B (-0.00% ) 21.3 kB
Card -- 62.6 kB ▼ -1 B (-0.01% ) 19.6 kB
CardContent -- 61.7 kB ▲ +1 B (+0.01% ) 19.4 kB
ClickAwayListener -- 3.85 kB ▼ -1 B (-0.06% ) 1.54 kB
Container -- 63 kB ▲ +1 B (+0.01% ) 19.8 kB
ExpansionPanelDetails -- 61.7 kB ▼ -1 B (-0.01% ) 19.4 kB
Fade -- 23.3 kB ▲ +1 B (+0.01% ) 8 kB
FormControlLabel -- 65.3 kB ▲ +1 B (0.00% ) 20.5 kB
FormGroup -- 61.8 kB ▲ +1 B (+0.01% ) 19.4 kB
FormHelperText -- 63 kB ▲ +1 B (+0.01% ) 19.8 kB
LinearProgress -- 65.1 kB ▲ +1 B (0.00% ) 20.4 kB
Link -- 66.4 kB ▲ +1 B (0.00% ) 21 kB
ListItemAvatar -- 61.9 kB ▲ +1 B (+0.01% ) 19.4 kB
NativeSelect -- 76.6 kB ▼ -1 B (-0.00% ) 24.2 kB
Popper -- 28.7 kB ▲ +1 B (+0.01% ) 10.3 kB
Portal -- 2.9 kB ▲ +1 B (+0.08% ) 1.3 kB
Radio -- 82.7 kB ▼ -1 B (-0.00% ) 26.2 kB
RootRef -- 4.21 kB ▲ +1 B (+0.06% ) 1.64 kB
SnackbarContent -- 63.3 kB ▲ +1 B (+0.01% ) 20 kB
Step -- 62.4 kB ▲ +1 B (+0.01% ) 19.6 kB
styles/createMuiTheme -- 16.5 kB ▲ +1 B (+0.02% ) 5.85 kB
SwipeableDrawer -- 91.6 kB ▲ +1 B (0.00% ) 28.7 kB
Table -- 62.3 kB ▲ +1 B (+0.01% ) 19.6 kB
TableBody -- 61.9 kB ▲ +1 B (+0.01% ) 19.4 kB
TextareaAutosize -- 5.09 kB ▼ -1 B (-0.05% ) 2.14 kB
useAutocomplete -- 12.7 kB ▲ +1 B (+0.02% ) 4.71 kB
@material-ui/core[umd] -- 314 kB -- 90.6 kB
AppBar -- 63.7 kB -- 20 kB
Badge -- 65.1 kB -- 20.3 kB
BottomNavigation -- 62.2 kB -- 19.5 kB
CircularProgress -- 63.9 kB -- 20.2 kB
colorManipulator -- 3.85 kB -- 1.52 kB
docs.landing -- 50.7 kB -- 13.4 kB
GridListTile -- 63.5 kB -- 20 kB
InputAdornment -- 64.8 kB -- 20.5 kB
ListItemIcon -- 61.9 kB -- 19.4 kB
ListItemText -- 64.7 kB -- 20.4 kB
Slide -- 25.3 kB -- 8.72 kB
StepContent -- 68.8 kB -- 21.6 kB
Tab -- 76.1 kB -- 24.1 kB
TableCell -- 63.8 kB -- 20.2 kB
Tabs -- 85.2 kB -- 27.1 kB
ToggleButton -- 75.9 kB -- 24.1 kB
useMediaQuery -- 2.5 kB -- 1.06 kB

Generated by 🚫 dangerJS against 5795edc

oliviertassinari
oliviertassinari reviewed Dec 6, 2019
View reviewed changes
docs/src/pages.js Outdated Show resolved Hide resolved
@oliviertassinari oliviertassinari changed the title [Alert] creates Alert component [Alert] Introduce new component Dec 6, 2019
@oliviertassinari oliviertassinari added the new feature New feature or request label Dec 6, 2019
@mbrookes mbrookes mentioned this pull request Dec 9, 2019
Closed
1 task
@oliviertassinari oliviertassinari added the component: alert This is the name of the generic UI component, not the React module! label Dec 9, 2019
@r3dm1ke
Copy link
Contributor

r3dm1ke commented Dec 12, 2019

I have a problem with a test failure in packages/material-ui/src/styles/createPalette.test.js:681:14 and I cannot figure out why. Can anyone please take a look, maybe I missed something?

@oliviertassinari
Copy link
Member

@r3dm1ke Is the fail related to the new colors in the palette? What if we handle it in another effort?

@dimitropoulos

This comment has been minimized.

Sign in to view
@oliviertassinari

This comment has been minimized.

Sign in to view
@dimitropoulos
Copy link
Member Author

cool - I removed AlertIcon in bb5d4c6 and instead used a startIcon API just like Button. I think it's a great improvement. It's actually much simpler this way and also has a new feature: the alert will now work just as well without an icon - which is cool. We don't need any "hasIcon" prop or anything.

@dimitropoulos
Copy link
Member Author

in 1d3d053 I did a similar treatment for the AlertClose. Now, the onClose functionality for that Close button works, as well as the new feature that the icon is customizable (but defaults to <Close /> if none is provided).

@dimitropoulos
Copy link
Member Author

in 1b50dd1 I addressed a functionality concern about wrapping in Typography for the contents of the alert. I changed the name to AlertContent to align with CardContent, DialogContent, and StepContent and also to signify that it's not just a description, necessairly since (now) someone can pass whatever they want in by specifying a variant of container on AlertContent (the default is text).

@dimitropoulos

This comment has been minimized.

Sign in to view
@r3dm1ke

This comment has been minimized.

Sign in to view
@oliviertassinari

This comment has been minimized.

Sign in to view
@mbrookes
Copy link
Member

I think that the AlertTitle, AlertDescription, AlertActions are better designed for the long tail of the use cases.

Do AltertTitle and AlertDescription add enough value to exist as standalone components, being just a thin wrapper around Typography? Your suggestion to just use Typography directly gives the developer more control of the styling (absent an alert component to follow in the MD spec).

@oliviertassinari
Copy link
Member

Good questions, I don't know.

@dimitropoulos

This comment has been minimized.

Sign in to view
@dimitropoulos

This comment has been minimized.

Sign in to view
@mbrookes

This comment has been minimized.

Sign in to view
@r3dm1ke

This comment has been minimized.

Sign in to view
@mbrookes

This comment has been minimized.

Sign in to view
@dimitropoulos

This comment has been minimized.

Sign in to view
@r3dm1ke

This comment has been minimized.

Sign in to view
@mbrookes

This comment has been minimized.

Sign in to view
mbrookes
mbrookes previously requested changes Dec 31, 2019
View reviewed changes
docs/src/pages/components/alert/DescriptionAlerts.js Outdated Show resolved Hide resolved
docs/src/pages/components/alert/DescriptionAlerts.js Outdated Show resolved Hide resolved
docs/src/pages/components/alert/DescriptionAlerts.js Outdated Show resolved Hide resolved
docs/src/pages/components/alert/DescriptionAlerts.js Outdated Show resolved Hide resolved
docs/src/pages/components/alert/DescriptionAlerts.tsx Outdated Show resolved Hide resolved
packages/material-ui-lab/src/Alert/Alert.js Outdated Show resolved Hide resolved
packages/material-ui-lab/src/Alert/Alert.js Outdated Show resolved Hide resolved
packages/material-ui-lab/src/Alert/Alert.js Outdated Show resolved Hide resolved
packages/material-ui-lab/src/Alert/Alert.js Show resolved Hide resolved
packages/material-ui-lab/src/Alert/Alert.js Show resolved Hide resolved
@dimitropoulos
Copy link
Member Author

looking like we're pretty close to a minimally viable component for the lab! is there anything else here we have in mind to do? @oliviertassinari @r3dm1ke @mbrookes?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 2, 2020 •
edited
Loading

I think that we are good to go. @mbrookes has uncovered an interesting topic on title vs Tooltip usage, but it's leaking from the Alert to the TablePagination and Autocomplete components. I don't think that it should block this pull request.

@oliviertassinari oliviertassinari merged commit 2f7b71c into mui:master Jan 3, 2020
@oliviertassinari
Copy link
Member

Thank you everybody for your time!

We will likely have a couple of follow-up pull requests, one step at the time :).

@dimitropoulos
Copy link
Member Author

woooohoooo! awesome! I'm more than happy to help with any followup work so please @oliviertassinari feel free to ping/assign me.

@dimitropoulos dimitropoulos deleted the alert-component branch January 3, 2020 15:16
@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 3, 2020 •
edited
Loading

@dimitropoulos Thanks! GitHub has this concept of code owner: https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners. It's something that we have never used yet. However, I think that it would be interesting to experiment. Would you be interested in being the code owner of the Alert component?
@mbrookes What do you think?

@mbrookes
Copy link
Member

mbrookes commented Jan 3, 2020

I didn't know about that feature. Worth trying!

@oliviertassinari
Copy link
Member

Awesome, let's experiment! I will handle this in my next batch of small changes (probably Sunday).

@oliviertassinari oliviertassinari mentioned this pull request Jan 5, 2020
Merged
@thisismydesign thisismydesign mentioned this pull request Jan 5, 2020
Closed
m4theushw pushed a commit to m4theushw/material-ui that referenced this pull request Jan 6, 2020
@dimitropoulos @m4theushw
[Alert] Introduce new component (mui#18702)
958955e
@snyk-bot snyk-bot mentioned this pull request Feb 15, 2020
Open
@snyk-bot snyk-bot mentioned this pull request Feb 25, 2020
Closed
@snyk-bot snyk-bot mentioned this pull request Apr 16, 2020
Merged
@oliviertassinari oliviertassinari mentioned this pull request Oct 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbrookes mbrookes mbrookes left review comments

@oliviertassinari oliviertassinari oliviertassinari approved these changes

@r3dm1ke r3dm1ke r3dm1ke left review comments

@merceyz merceyz merceyz requested changes

Assignees
No one assigned
Labels
component: alert This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Snackbar] Enable usage with the Alert [Alert] Add new component
6 participants
@dimitropoulos @mui-pr-bot @r3dm1ke @oliviertassinari @mbrookes @merceyz