Skip to content
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

Repair and refine embed block styles #14648

Closed

Conversation

m-e-h
Copy link
Member

@m-e-h m-e-h commented Mar 26, 2019

Description

editor.scss

  • corrected the attribute selector from [data-type="core/embed"] to [data-type^="core-embed"] and move these style rules from style.scss the more appropriate editor.scss.

style.scss

  • Reduced specificity by un-nesting and removing unnecessary selectors.
  • Added 100% width to the iframe for alignright and alignleft. Previously, on embeds without an aspect-ratio, floats weren't responsive and the iframe would overflow it's container. (If using normalize.css this wouldn't be noticed)

How has this been tested?

Tested visually with several themes including TwentyNineteen.

Types of changes

Bug fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

padding-top: 50%; // Default to 2:1 aspect ratio.
}
// Add responsiveness to embeds with aspect ratios.
.wp-embed-responsive .wp-has-aspect-ratio {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To save some bytes, switched to .wp-has-aspect-ratio rather than listing all the different aspect-ratios.
Any reason not to do this?

@@ -1,3 +1,11 @@
// Apply max-width to floated items that have no intrinsic width
.block-editor-block-list__block[data-type^="core-embed"][data-align="left"] .block-editor-block-list__block-edit,
.block-editor-block-list__block[data-type^="core-embed"][data-align="right"] .block-editor-block-list__block-edit {
Copy link
Member Author

@m-e-h m-e-h Apr 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not completely clear on this though. Maybe core/embed is correct. I just now saw where thats used for the generic embed? Everything else appears to use something like core-embed/youtube.

Are the generic embeds the only thing that doesn't have an intrinsic width?

@swissspidy swissspidy added the [Block] Embed Affects the Embed Block label May 28, 2019
@jasmussen
Copy link
Contributor

Hello @m-e-h, apologies for not being able to look at this back when it was fresh. It is a good PR and should have landed ages ago.

If you have the time and energy, I have tested this branch and it works as intended and should be good to merge if you rebase. In absence of that, I understand you may be busy, I took the liberty of creating #21599 which is simply a rebased copy of this branch, with props to you. Let me know if you'd like to rebase this and get it in, or whether we should move on with the other! Thanks again.

@jasmussen
Copy link
Contributor

I'm closing this one as I managed to port your code to #21599 and merge it! Thanks again.

@jasmussen jasmussen closed this May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Embed Affects the Embed Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants