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
Editor: Update behavior of consecutive snackbars #13190
Conversation
Size Change: -32 B (0%) Total Size: 2.73 MB ℹ️ View Unchanged
|
Plugin builds for 3c34bea are ready 🛎️!
|
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.
The new behavior looks good to me 👍
Just one question about the key
attribute
packages/design-system/src/components/snackbar/snackbarContainer.tsx
Outdated
Show resolved
Hide resolved
| Pick<SnackbarNotification, 'id'> | ||
| Pick<SnackbarNotification, 'id'>[] | ||
) => { | ||
setNotifications((currentNotifications) => | ||
currentNotifications.filter((item) => { | ||
if (Array.isArray(toRemove)) { | ||
return !toRemove.find(({ key }) => key === item.key); | ||
return !toRemove.find(({ id }) => id === item.id); | ||
} | ||
return item.key !== toRemove.key; | ||
return item.id !== toRemove.id; |
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.
key
is actually used in at least one place:
web-stories-wp/packages/story-editor/src/app/media/useUploadMedia.js
Lines 106 to 116 in be47067
if (isTranscoding) { | |
showSnackbar({ | |
message: __('Optimizing file…', 'web-stories'), | |
dismissible: true, | |
key: 'video-optimization', | |
}); | |
} else { | |
removeSnack({ | |
key: 'video-optimization', | |
}); | |
} |
Should we just remove key
there because we only show 1 notification now anyway?
If so, we should probably remove key
entirely from the SnackbarNotification
type.
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.
instead of using key
here we can pass id
in showSnackbar
. This will overwrite the default id
given using uuid
.
In case we want to show more than one snack bar in the future, notifications can be still deleted using removeSnakbar
and a known id.
Context
Updates consecutive snack bar behavior to be more inline with Material design guidelines
Summary
SnackbarProvider
'sremove
function was removing notifications on the basis ofkey
which was not a valid key. This was causing all snack bars to be removed when thenotifications
list hit its max limit.SnackbarContainer
which was causing enter animation not to apply.SnackbarContainer
to show only one snack bar at a time.Relevant Technical Choices
To-do
User-facing changes
Before -
Screen.Recording.2023-04-10.at.2.10.48.PM.mov
After -
Screen.Recording.2023-04-10.at.2.08.01.PM.mov
Testing Instructions
This PR can be tested by following these steps:
Reviews
Does this PR have a security-related impact?
Does this PR change what data or activity we track or use?
Does this PR have a legal-related impact?
Checklist
Type: XYZ
label to the PRFixes #12845