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

#2504 Fix for aligned embed videos #3636

Merged
merged 1 commit into from
Nov 28, 2017

Conversation

Mathiu
Copy link
Contributor

@Mathiu Mathiu commented Nov 23, 2017

Description

Added static width on aligned video embeds, otherwise they have no width. Also made sure it works on mobile.

How Has This Been Tested?

Tested on PC Chrome 62 & Firefox 58 (beta5)

Types of changes

CSS

Checklist:

  • My code is tested (partially i guess?).
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

.editor-block-list__block[data-type="core/embed"] {
&[data-align="left"],
&[data-align="right"] {
max-width: 100%;
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? Or at least, should it be applying so specifically to the embed block type? Why aren't other aligned blocks susceptible to this?

// Alignments
&[data-align="left"],
&[data-align="right"] {
// Without z-index, won't be clickable as "above" adjacent content
z-index: z-index( '.editor-block-list__block {core/image aligned left or right}' );
max-width: 370px;
}

In my testing, it seems to work fine without this specific style.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, from what I can tell there's an issue of specificity where the max-width: 700px of .editor-visual-editor .editor-block-list__block takes precedence over the max-width: 370px of .editor-block-list__block[data-align="left"]

.editor-visual-editor .editor-block-list__block {
margin-left: auto;
margin-right: auto;
max-width: $visual-editor-max-width + ( 2 * $block-mover-padding-visible );
&[data-align="wide"] {
max-width: 1100px;
}
&[data-align="full"] {
max-width: 100%;
}
&[data-align="full"],
&[data-align="wide"] {
.editor-block-contextual-toolbar {
@include break-small() {
width: $visual-editor-max-width + 2; // 1px border left and right
}
margin-left: auto;
margin-right: auto;
}
}
}

Might have been a regression accidentally introduced with changes of #3546.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's only necessary to make aligned embed responsive. It's visible only at < 320px, I'm not sure if it should be supported.

Regarding specificity issue, the max-width is not related to fixing aligned embed videos, so it's probably unnecessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants