Fix non responsive embed iframes styles #30254
Closed
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
youknowriad
added
[Type] Enhancement
14 tasks
|
Size Change: +2.52 kB (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
|
The goal of this one is cool, but I do worry a little bit about what auto will do to embeds here. What we've found in the past with some embeds is that they will try to size themselves to the width of the embed, i.e. if you set the width to 100% as it was before, embeds inside will size themselves to that as well. With an auto width, I worry that means they'll collapse entirely in a weird race condition. Here's what a non-responsive YouTube video looks like:
Since this style is tied to the embed block, I think it be one where we can have a better defalt than the browser, compared to if we had just a generic iframe rule. In this case, I think the "before" is actually the preferable state:
Though of course that isn't consistent with the frontend:
Do we have any alternatives to setting the width to auto? Could we simply not set any width at all? |
|
We have two alternatives:
|
|
I'd be ready to approve both of those options, though I have a small personal preference for the latter. To an extent you could consider the embed as a "block level element", which by nature, sort of, defaults to 100% width. |
|
Updated to make all iframe 100%. This could use some testing now :) |
|
This approach seems problematic for embeds that set a specific width. For example Spotify:
The markup on this post is useful for testing: https://theamdemo.wordpress.com/2018/10/24/gutenberg-embed/ |
|
Unfortunately, this means a compromise is needed here:
I'm going to close this PR for now, feel free to give it another spin later though. |
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.
Related to #29976
So it turns out by default browsers give iframe a width of 500px and in the editor we default to 100% for embed iframe meaning the editor doesn't look like the frontend.
I also noticed that we have some iframe that have an "aspect ratio" applied and these are already 100% wide in both editor and frontend.
So this PR makes the width equal to
auto(so width of its content) for iframes that don't use the aspect ratio styles.This is an area I'm not very familiar with so I'd love some testing of different embeds producing iframe (YouTube, amazon, Spotify...) :)