[Snackbar] Match the new specification #15122

oliviertassinari · 2019-03-30T17:06:33Z

Closes #14785. I had a few changes to make to match the specification.

The new specification: https://material.io/design/components/snackbars.html.

Breaking changes

Dimensions changed.
Default transition is Grow instead of Slide.

mui-pr-bot · 2019-03-30T17:11:33Z

@material-ui/core: parsed: +Infinity% , gzip: +Infinity%
@material-ui/lab: parsed: +Infinity% , gzip: +Infinity%
@material-ui/styles: parsed: +Infinity% , gzip: +Infinity%
@material-ui/system: parsed: +Infinity% , gzip: +Infinity%

Details of bundle changes.

Comparing: 03e01e3...e735fe2

bundle	parsed diff	gzip diff	current parsed	current gzip
@material-ui/core	+Infinity% 🔺	+Infinity% 🔺	349,848	89,858
@material-ui/core/Paper	+Infinity% 🔺	+Infinity% 🔺	67,853	19,820
@material-ui/core/Paper.esm	+Infinity% 🔺	+Infinity% 🔺	60,184	18,563
@material-ui/core/Popper	+Infinity% 🔺	+Infinity% 🔺	30,463	10,530
@material-ui/core/styles/createMuiTheme	+Infinity% 🔺	+Infinity% 🔺	17,364	5,726
@material-ui/core/useMediaQuery	+Infinity% 🔺	+Infinity% 🔺	2,469	1,041
@material-ui/lab	+Infinity% 🔺	+Infinity% 🔺	147,758	43,621
@material-ui/styles	+Infinity% 🔺	+Infinity% 🔺	53,105	15,429
@material-ui/system	+Infinity% 🔺	+Infinity% 🔺	17,136	4,528
Button	+Infinity% 🔺	+Infinity% 🔺	87,928	26,054
Modal	+Infinity% 🔺	+Infinity% 🔺	82,035	24,552
colorManipulator	+Infinity% 🔺	+Infinity% 🔺	3,232	1,299
docs.landing	+Infinity% 🔺	+Infinity% 🔺	49,914	10,815
docs.main	+Infinity% 🔺	+Infinity% 🔺	645,455	200,410
packages/material-ui/build/umd/material-ui.production.min.js	+Infinity% 🔺	+Infinity% 🔺	298,251	82,662

Generated by 🚫 dangerJS against e735fe2

eps1lon · 2019-03-31T08:45:59Z

Added detailed changes to the PR description. Could you correct them if they're wrong/incomplete? Helps people evaluating that change.

eps1lon

https://deploy-preview-15122--material-ui.netlify.com/demos/snackbars/#consecutive-snackbars is now outdated. There is no more downwards/upwards movement.

updated, thank you

joshwooding

I prefer the old look of the 'Snackbars and floating action buttons (FABs)' demo which matches the other demos on the page but LGTM

joshwooding

Looks like the comment for the Slide component needs to be changed https://github.com/mui-org/material-ui/blob/bbfbb0ca6917475e196efc21e826e868d630e75e/packages/material-ui/src/Slide/Slide.js#L73

mbrookes · 2019-03-31T23:04:53Z

For the 'Snackbars and floating action buttons (FABs)' I think it would be preferable to show this at mobile width, so that it matches the new behaviour shown in the spec, regardless of the width of the browser. At first glance it is hard to see what is being demonstrated.

Alternatively, a toggle to switch the width, although that would muddy the distinction between what is required to implement the behavior, vs what is required to demo it.

Also, the app bar title "Out of my way" no longer makes sense, as the snackbar isn't now "pushing" the Fab out of the way.

updated

joshwooding

I still prefer the old look of the 'Snackbars and floating action buttons (FABs)' demo which matches the other demos on the page (text button on a grey background) but LGTM

mbrookes

The "Open snackbar" button didn't used to have a background, in order to to separate it from the "App":

Now it looks like it's a part of the "App", with the AppBar pushed down:

The previous width looked more like that of a mobile app.
Previously mentioned:

Also, the app bar title "Out of my way" no longer makes sense, as the snackbar isn't now "pushing" the Fab out of the way.

oliviertassinari · 2019-04-01T19:11:39Z

~~@mbrookes I'm removing the demo. I think that it's simpler this way.~~

I have a better idea.

oliviertassinari · 2019-04-01T20:15:10Z

@mbrookes Could you confirm it's good now?

mbrookes

Not all a direct result of this PR, but here are few observations from looking at the rest of the demos:

Customized:
The "Open success snackbar" button isn't obviously a button. Would an outlined outlined button be better?

Positioned:
After opening the first snackbar, the rest don't get an opening transition.

Control Direction:
Says "Slide is the default transition." This is no longer the case.

Change Transition:
Demos Slide, which is already shown in the Control Direction demo. Demos Grow, which is the default transition.

notistack:
The revised spec says not to stack snackbars, so if we're keeping this demo, the text should perhaps mention that.

oliviertassinari · 2019-04-01T21:16:30Z

@mbrookes Thanks, it should be taken care of.

mbrookes · 2019-04-01T23:21:30Z

Just this one left:

Change Transition:
Demos Slide, which is already shown in the Control Direction demo. Demos Grow, which is the default transition.

oliviertassinari · 2019-04-02T07:50:13Z

Why do you want to remove it? The concerns are different, one is about changing the transition, the other about changing the Slide direction.

eps1lon · 2019-04-02T08:31:40Z

If the default transition is not Slide anymore is it really that important to have an example about the direction? This is more a concern for Slide demos. direction isn't even an apparent prop.

Change Transition:
[...] Demos Grow, which is the default transition.

I would keep that there (maybe add a "this is default" hint) for easier comparison.

oliviertassinari · 2019-04-02T08:35:45Z

I would keep that there (maybe add a "this is default" hint) for easier comparison.

I'm on it 👍

If the default transition is not Slide anymore is it really that important to have an example about the direction?

I'm wondering too, but I believe it's something common.

mbrookes · 2019-04-02T12:46:06Z

I would keep that there (maybe add a "this is default" hint) for easier comparison.

Except that it isn't actually just showing the default, it's "replacing" the default transition (Grow) with Grow. As a code example it's... odd.

eps1lon · 2019-04-02T13:05:18Z

I would keep that there (maybe add a "this is default" hint) for easier comparison.

Except that it isn't actually just showing the default, it's "replacing" the default transition (Grow) with Grow. As a code example it's... odd.

Sure we don't have to explicitly define it. That's rather nitpicky though. I would rather focus on the actual demo: Do you agree that we should include the defeault transition in the demo?

mbrookes · 2019-04-02T16:51:04Z

I lean towards the simplest demo that shows the particular feature or function, in this case using a custom transition. But if you believe having the side-by-side comparison is helpful, then sure.

shilangyu · 2019-08-20T10:00:06Z

Sorry for commenting on an old PR, but was this the PR that removed full-width snackbars on mobile?

oliviertassinari · 2019-08-20T10:06:51Z

Yes

oliviertassinari added design: material This is about Material Design, please involve a visual or UX designer in the process component: snackbar This is the name of the generic UI component, not the React module! labels Mar 30, 2019

oliviertassinari force-pushed the update-snackbar-implementation branch 4 times, most recently from b89cf43 to 61488d4 Compare March 30, 2019 17:44

oliviertassinari added breaking change PR: good for merge labels Mar 30, 2019

eps1lon removed the PR: good for merge label Mar 31, 2019

eps1lon previously requested changes Mar 31, 2019

View reviewed changes

joshwooding approved these changes Mar 31, 2019

View reviewed changes

oliviertassinari added the PR: good for merge label Mar 31, 2019

joshwooding previously requested changes Mar 31, 2019

View reviewed changes

oliviertassinari removed the PR: good for merge label Mar 31, 2019

oliviertassinari force-pushed the update-snackbar-implementation branch 2 times, most recently from 2755c00 to 45bf57f Compare April 1, 2019 08:09

oliviertassinari requested review from joshwooding and mbrookes April 1, 2019 08:22

oliviertassinari added the PR: good for merge label Apr 1, 2019

joshwooding approved these changes Apr 1, 2019

View reviewed changes

mbrookes reviewed Apr 1, 2019

View reviewed changes

oliviertassinari force-pushed the update-snackbar-implementation branch 2 times, most recently from b066c64 to 6953f95 Compare April 1, 2019 19:19

mbrookes reviewed Apr 1, 2019

View reviewed changes

oliviertassinari added 6 commits April 3, 2019 00:07

[Snackbar] Match the new specification

244f6cf

sebastian review

9641171

Josh & Matt reviews

e6fa198

matt review

cd1d2a3

more matt feedback

217e68f

more feedback

9683799

oliviertassinari force-pushed the update-snackbar-implementation branch from a6cd990 to a8a9898 Compare April 2, 2019 22:08

rebase

e735fe2

oliviertassinari force-pushed the update-snackbar-implementation branch from a8a9898 to e735fe2 Compare April 2, 2019 22:52

oliviertassinari merged commit 25ea3bf into mui:next Apr 3, 2019

eps1lon mentioned this pull request Apr 17, 2019

RFC: Material-UI v4 breaking changes #13663

Closed

56 tasks

oliviertassinari deleted the update-snackbar-implementation branch August 20, 2019 10:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Snackbar] Match the new specification #15122

[Snackbar] Match the new specification #15122

oliviertassinari commented Mar 30, 2019 •

edited

Loading

mui-pr-bot commented Mar 30, 2019 •

edited

Loading

eps1lon commented Mar 31, 2019

eps1lon left a comment

joshwooding left a comment •

edited

Loading

joshwooding left a comment

mbrookes commented Mar 31, 2019

joshwooding left a comment

mbrookes left a comment

oliviertassinari commented Apr 1, 2019 •

edited

Loading

oliviertassinari commented Apr 1, 2019

mbrookes left a comment •

edited

Loading

oliviertassinari commented Apr 1, 2019

mbrookes commented Apr 1, 2019

oliviertassinari commented Apr 2, 2019 •

edited

Loading

eps1lon commented Apr 2, 2019

oliviertassinari commented Apr 2, 2019

mbrookes commented Apr 2, 2019

eps1lon commented Apr 2, 2019

mbrookes commented Apr 2, 2019 •

edited

Loading

shilangyu commented Aug 20, 2019

oliviertassinari commented Aug 20, 2019

[Snackbar] Match the new specification #15122

[Snackbar] Match the new specification #15122

Conversation

oliviertassinari commented Mar 30, 2019 • edited Loading

Breaking changes

mui-pr-bot commented Mar 30, 2019 • edited Loading

eps1lon commented Mar 31, 2019

eps1lon left a comment

Choose a reason for hiding this comment

joshwooding left a comment • edited Loading

Choose a reason for hiding this comment

joshwooding left a comment

Choose a reason for hiding this comment

mbrookes commented Mar 31, 2019

joshwooding left a comment

Choose a reason for hiding this comment

mbrookes left a comment

Choose a reason for hiding this comment

oliviertassinari commented Apr 1, 2019 • edited Loading

oliviertassinari commented Apr 1, 2019

mbrookes left a comment • edited Loading

Choose a reason for hiding this comment

oliviertassinari commented Apr 1, 2019

mbrookes commented Apr 1, 2019

oliviertassinari commented Apr 2, 2019 • edited Loading

eps1lon commented Apr 2, 2019

oliviertassinari commented Apr 2, 2019

mbrookes commented Apr 2, 2019

eps1lon commented Apr 2, 2019

mbrookes commented Apr 2, 2019 • edited Loading

shilangyu commented Aug 20, 2019

oliviertassinari commented Aug 20, 2019

oliviertassinari commented Mar 30, 2019 •

edited

Loading

mui-pr-bot commented Mar 30, 2019 •

edited

Loading

joshwooding left a comment •

edited

Loading

oliviertassinari commented Apr 1, 2019 •

edited

Loading

mbrookes left a comment •

edited

Loading

oliviertassinari commented Apr 2, 2019 •

edited

Loading

mbrookes commented Apr 2, 2019 •

edited

Loading