-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Update video placeholder style. #44215
Conversation
Size Change: +389 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
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.
Hey Joen @jasmussen
Testing the visual using a local site running Twenty Twenty Two.
Gutenberg plugin 14.1
After using PR Gutenberg build.
This very much improves the current lack of placeholder in the video block.
Thank you for working on this Joen!
As you say this is a good starting point for further explorations at another time.
Thanks for the review! I think I'll pause this one and pick it back up when #44190 is in a place where it can land, that will help better differentiate it from other similar placeholders. |
3d47dd8
to
ac9afa7
Compare
Rebased this one to see it in the new style (see also #44460), and I'm seeing some glitchiness: 1st block is an image block. I don't know why it has that gray background, when the other two don't. Second is this video block, third is featured image. @jameskoster do you have any insights as to why the layers are wrong in this version, and/or why the image block is light gray in this yellow scheme? Feel free to push to this branch if you find a fix. |
I'm not seeing that: As for the grey overlay on the video block, it's caused by the pseudo element which needs to be hidden when the block is selected. Here's how I did it for the image block: afb0dcb |
I just pushed the same fix here. |
Thank you, that works well for me! About the weird gray backdrop, I've found a way to reproduce:
Gif: The weird thing is: this is only an issue with the Image block, it doesn't appear to be an issue with this video block. Can you reproduce, and do you have any idea why this might be? |
I'm afraid not. It's fine both on trunk and this PR for me 🤔 this.pr.mp4trunk.mp4 |
😩 Chrome? Safari? Firefox? |
Working fine in all three for me. |
Alright, it seems clear that the backdrop filter is having some effects for me. #44497 (comment) — but since that's separate, I'll land this one when the checks pass. |
What?
Followup to #43180. See also #44190. Updates the placeholder of the Video block to start out dashed. Instead of showing the resting state of the video block as the dark-bordered classic placeholder, it now shows a dashed outline until you select it.
Before:
After:
We'll likely want an icon to denote the difference between placeholders, but perhaps this one is good enough to land as a starting point?
Why?
For patterns and templates, including a video block can be a good way to suggest a particular use case. But the descriptions quickly become repetitive in patterns where you may have multiple video blocks. By showing these on select, we better enable this use case.
Testing Instructions
Insert a video block and observe its resting and selected states.