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
Fix 9:16 aspect ratio styling #25972
Conversation
|
Size Change: 0 B Total Size: 1.19 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.
My goodness, thanks so much for finding this typo — what even is a 9/6 aspect ratio!? 3/2 could be something, but 9/6 isn't, yet I still managed to calculate the padding based off of it. I must've been very tired that day, thanks for catching it.
Description
I noticed that even though there are these
ASPECT_RATIOSdefined:gutenberg/packages/block-library/src/embed/constants.js
Lines 1 to 11 in 78585d6
There was no style rule defined for
.wp-embed-aspect-9-16. Instead there is apparently either an old style rule for.wp-embed-aspect-9-16or it was entered with a typo:gutenberg/packages/block-library/src/embed/style.scss
Lines 77 to 79 in 78585d6
This typo appears to have been introduced in #21599.
This is closely related to #18518. Since there is no style rule for
.wp-embed-aspect-9-16in current Gutenberg, the fallback aspect ratio of 2:1 is used:gutenberg/packages/block-library/src/embed/style.scss
Lines 39 to 43 in 78585d6
When adding a Spotify block, the aspect ratio being detected is 9:16. Before the style rule is fixed, it gets the 2:1 fallback. But after the fix, the height gets quite excessive:
The issue appears to be due to the Spotify
iframehaving aheightthat is just a little larger than thewidth:There appears to be a missing aspect ratio for 3:4 to invert the 4:3. This is not included in the scope of this PR.
How has this been tested?
Add a Spotify embed, e.g. https://open.spotify.com/album/4RuzGKLG99XctuBMBkFFOC
Types of changes
Fixing styles, which reveal a deeper issue with
getClassNames()in theembed/util.js.Checklist: