-
Notifications
You must be signed in to change notification settings - Fork 4.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
Editor: Avoid rendering snackbar notices twice in list data views #61676
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: 0 B Total Size: 1.75 MB ℹ️ View Unchanged
|
It's interesting that this PR actually breaks the e2e test for "activating theme from live previews". Basically snackbars are not visible there when activating a theme. First I'd say that lack of snackbar there is a smaller bug than double snackbars in list views (which are a more common flow). That said, It highlights something interesting: Where should we render and position the snackbars? This question doesn't have a good answer today in the site editor, especially outside "edit" mode. I would argue that we shouldn't render the snackbars "within the frame" because the frame is a big giant button basically and there shouldn't be any actionable items within that frame. Also, we have pages that have the "frame", others that have "content" + "frame" and other pages that have only "content". Any thoughts on that @jasmussen @jameskoster @scruffian |
@jameskoster so what do you think we should do here? Ship this or wait until we figure out the holistic approach to snackbars ? |
I opened a PR for this, as it's on trunk too. Snackbars feel to me that they should be in a single place for a page and not coupled with more than one component.. Having said that, maybe for a first step we could conditionally show the snackbars from |
@youknowriad I think we should merge this as it fixes the egregious duplicate snackbars issues. Separately it would be nice to update Snackbar position as described in my previous comment. |
Maybe I'm misreading the comment but would you have a mockup for that or something, where would you put the snackbars? |
Probably in the bottom left. The important thing is they should be positioned relative to the viewport rather than any UI element. This is what we have, note how the snackbar moves: snackbar.mp4I would expect them to be static. Rough mockup: snackbar.mp4This may need more design feedback though, as it would also involve changing the snackbar background color so that it is visible against the dark sidebar. |
Ok I'll explore that in a separate PR. |
Alternative PR created here #61756 |
closes #60457
What?
When manipulating the pages in the "list view", it can result in snackbar notices showing up twice: outside and inside the frame. This PR only shows the notices inside the frame in "edit" mode.
Testing Instructions
1- Open the "pages" menu in the site editor
2- Rename a page from the actions menu
3- Notice that a snackbar shows up (but only once)