-
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
Snackbar: Update snackbar position when list view or inserter opened #47298
Conversation
I would love to disable the snackbar animations when the position is changing to stop them flying around, but I can't see an obvious way to selectively do this. Update: animations are now disabled. |
Size Change: +111 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
Flaky tests detected in ad87863. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4116861720
|
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 would love to disable the snackbar animations when the position is changing to stop them flying around, but I can't see an obvious way to selectively do this.
Hmm. Could we set transition-property: opacity
on the snackbar instead of transition-property: all
? Assuming that opacity is the only thing we actually want animated.
Unfortunately it's not as easy as that because there is a height transition when the notice is first added. I've debugged this, and it seems as though the position transform is not coming from the |
I'm having a hard time isolating where this animation comes from. I don't think this PR can merge without it being removed however, it looks too sloppy. |
Fixed! This was coming from the I don't believe this option is necessary since the snackbar notifications are outside the flow of the rest of the page. I did not notice any change in performance, even with multiple snackbar notices present. |
03b672b
to
7b70c84
Compare
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.
Nice! Good solve.
I personally really like this approach. Fixes the issue and feels quite natural.
Maybe let's confirm with @jasmussen or @SaxonF.
Thanks for the review @noisysocks! I've fixed up those SCSS issues. Also added @SaxonF for a design review. 👍 |
The animation of the Snackbars when the left-hand panel(s) appear or disappear is a bit distracting, is it possible to remove that? I don't know if it's feasible, but it may also be worth 'pinning' the snackbars to the frame rather than the entire viewport. That would eliminate the issue we have in the browse view where they:
|
Agreed, this is actually gone in the latest. I will post a video in a bit.
I haven't looked at the issue with the browse view, but looks like the same problem. Let me work on this and see if I can adjust it there too. 👍 |
27da118
to
ad87863
Compare
I've had a look at this, and it's going to be a fair amount of extra work to structure it this way. The snackbars would need to be rendered within the frame to stay pinned. I think this would be good as a follow up PR as it's related but still a separate problem than it obscuring inserter UI which is already fixed in this PR. |
Nice that's better. I noticed the Snackbars are hidden if you click the W to open the menu, was that intentional? |
I checked and the snackbars are hidden if you click the W menu in trunk. Are you seeing something different? |
No, that's exactly what I'm seeing. My point is that I'm not sure we should actively hide them because there are some actions that occur outside of the editor and produce a Snackbar, such as deleting a custom template. Ideally the Snackbar is visible, but pinned to the bottom corner of the canvas frame: |
…ning/closing the sidebar. Removing does not seem to affect performance, even with multiple snackbar notices present.
ad87863
to
62bb0fd
Compare
Rebased to start looking at this one again. |
@apeatling A bit of a connection with this issue on snackbar placement relative to breadcrumbs/footer: #53198 |
What?
Spin off from #47199 to address the original concern in #45237 (review).
This change will move snackbars so that their left edge is always lined up with the left edge of the editor canvas (plus padding). This change affects both the post and site editors.
Why?
The snackbards interfere with the interface elements in the inserter and list view depending on state. This often disrupts the flow of pattern insertion as one example (see videos).
By shifting the snackbars over to match the left edge of the canvas (that also moves), we can avoid this issue.
How?
Adding
is-inserter-open
andis-list-view-open
classes to the editor containers on both the post and site editor. Then adjusting the left padding of the snackbar container based on the$nav-sidebar-width
.Testing Instructions
Screenshots or screencast
Before: snackbars get in the way of pattern insertion flows
before.mp4
After: snackbars move out of the way and do not interfere with flows (update: there's no animation making them slide around in the latest version)
after.mp4