-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
[SnackbarUnstyled] Create component and useSnackbar
hook
#33227
[SnackbarUnstyled] Create component and useSnackbar
hook
#33227
Conversation
MUI Base
and use it within Material UI Snackbar
componentMaterial UI Snackbar
component
I've finished my draft of the documentation. @ZeeshanTamboli let me know if the info I've added is accurate! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've finished my draft of the documentation. @ZeeshanTamboli let me know if the info I've added is accurate!
@samuelsycamore It looks great! 👌 I have pushed a few commits to fix docs and left two comments.
|
||
A snackbar provides users with a brief message about app processes without interrupting their activity or experience. | ||
|
||
The `SnackbarUnstyled` component is built to appear temporarily in the corner of the screen to inform users of an action that the app is taking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily in the corner only, it can be triggered in the center as well. See Top-center, bottom-center: https://mui.com/material-ui/react-snackbar/#positioned-snackbars in @mui/material
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, I forgot about that! I really like the demo for this in the Material UI docs. Do you think it would be worthwhile to recreate it with Base? (I do 😁)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not recreate it with Base since there is no anchorOrigin
prop and a unstyled Snackbar can actually be placed anywhere on the screen.
I just wanted to point out the placement convention of a Snackbar component that it can be aligned on center as well in applications, not just on corners.
@ZeeshanTamboli I've revised the Introduction and expanded on the Basics section, explaining what differentiates a snackbar from other alerts, and pointing out the I was a little hesitant to use Material Design as a source for the definition of a snackbar (since Base should theoretically be independent from MD / Material UI), but it seems like the definitive source on the topic. |
@samuelsycamore It looks great! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor comments, otherwise looks good to me 👍
|
||
Use the `components` prop to override any interior slots in addition to the root: | ||
|
||
```jsx | ||
<SnackbarUnstyled components={{ Root: 'span' }} /> | ||
``` | ||
|
||
:::warning | ||
If the root element is customized with both the `component` and `components` props, then `component` will take precedence. | ||
::: | ||
|
||
Use the `componentsProps` prop to pass custom props to internal slots. | ||
The following code snippet applies a CSS class called `my-snackbar` to the root slot: | ||
|
||
```jsx | ||
<SnackbarUnstyled componentsProps={{ root: { className: 'my-snackbar' } }} /> | ||
``` | ||
|
||
:::warning | ||
Note that `componentsProps` slot names are written in lowercase (`root`) while `components` slot names are capitalized (`Root`). | ||
::: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this section provides any value for the deverloper as the component doesn't have additional internal slots. cc @samuelsycamore should we stip this from the components that support only the root slot?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding componentsProps
, we also have componentProps.clickAwayListener
in this component. We can remove the components
prop part as there is only Root
slot. I'll leave it to @samuelsycamore .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm that's a good question @mnajdova. I like the uniformity of including this information in all of the component docs, but you're right that it is overkill for a component like this one. Is there a use case where a developer would want/need to use components
here instead of component
? My only concern would be that devs might think you can't use components
on a component like this one, even though you can (but probably don't need to).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's leave it as is for now, until we decide on the components
-> slots
rename, as well as what will the name of the component
prop be in MUI Base. We can revisit it later.
To answer your question @samuelsycamore there is no difference in MUI Base between component
vs components.Root, the first one is a shorthand. However, this would behave a bit differently in Material UI and Joy UI, because there the
component` prop acts like the emotion's "as" prop, which means it will only change the render HTML element, instead of replacing everything with the new component (for example, the styles and everything else created in the component will still apply. This is why we have a discussion going to decided on the API :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got a couple of smaller remarks. Other then that, I think it's good to be merged in.
packages/mui-base/src/SnackbarUnstyled/snackbarUnstyledClasses.ts
Outdated
Show resolved
Hide resolved
packages/mui-base/src/SnackbarUnstyled/snackbarUnstyledClasses.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @ZeeshanTamboli, I will try to include this in the next release :)
Great execution @ZeeshanTamboli and sorry for the delay with the review |
Part of mui/base-ui#10.
Also part of #31991 (Stacked snackbars). See #31991 (comment).
Once merged, I will use this for Stacked snackbars as well.
Docs preview: https://deploy-preview-33227--material-ui.netlify.app/base/react-snackbar/