Browse Mode: Add snackbar notices #50794
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
scruffian
added
the
[Feature] Site Editor
|
Size Change: +260 B (0%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 89b9581. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5025918587
|
jeryj
reviewed
May 19, 2023
There was a problem hiding this comment.
It's working well. The 1.001 definitely feels hacky, but I'm not familiar enough with it to know what other options might be, and this is definitely better than no notice. I did find another issue: when you dismiss the snackbar the animation starts from the wrong location. I assume it's unrelated to this one.
I ran into this in a previous PR. It's coming from the |
|
If I remember properly, the reason the snackbars were not in the "edit" mode is that sometimes they can persist when leaving "edit" mode to "view" mode. I'm fine if that's considered "ok" though. |
scruffian
force-pushed
the
add/snackbars-in-browse-mode
branch
from
89b9581
to
71e7af1
Compare
|
Ok, I've made the following changes to this PR:
IMO having the same snackbar notices in both contexts is a good, since they even stay in the same position, it helps to orient the user. |
jeryj
approved these changes
May 23, 2023
|
@scruffian @jeryj I noticed this PR seems to break block toolbars in the site editor, they appear blurry (at least on my not particularly high pixel density display) and don't follow the block position like they used to: The scale explains the blur, but not sure on the rest of it. Hope you don't mind, but I think I'll put together a revert of this PR as it's quite a bad bug. edit: Revert PR here - #50937 |
talldan
added a commit
that referenced
this pull request
May 25, 2023
|
Also seems to have caused this issue with top toolbar mode, so that's something else to test for when re-implementing: |
cbravobernal
pushed a commit
that referenced
this pull request
May 25, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Adds Snackbar notices to browse mode.
Why?
Setting a transform property (in this case scale) on an element makes it act as a containing block for its descendants. This means that the snackbar notices inside this component are repositioned to be relative to this element. To avoid the snackbars jumping about we need to ensure that a transform property is always set. Setting a scale of 1 is interpred by framer as no change, so once the animation completes the transform property of this element is set to none, thus removing its role as a container block.
How?
We set the initial scale of this element to 1.0001 so that there is always a transform property set. If we set the initial scale to less than 1.001 then JavaScript rounds it to 1 and the problem reoccurs.
Other solutions are to set a perspective or filter property on the same element in CSS, which would potentially be simpler but also much less clear.
The issue this is solving (#50175) suggested moving this to the centre, but it's not clear whether we mean the centre of the window or of the site frame. This solution is fairly simple and gets the snackbars working. We can follow up with a design iteration to improve things.
Testing Instructions
Testing Instructions for Keyboard
Follow the steps as above and confirm that you are given notices.
Screenshots or screencast