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

Fixed some Embed's width in the editor. #6212

Merged
merged 1 commit into from Apr 18, 2018

Conversation

Projects
None yet
3 participants
@jorgefilipecosta
Member

jorgefilipecosta commented Apr 16, 2018

Description

This Pr corrects a style rule so the non-video embed's also become responsive (videos like youtube already were).

Fixes: #5757

How has this been tested?

Verify the problem described in #5757 does not happen and the embed correctly resizes to the available space.

@jasmussen

This comment has been minimized.

Contributor

jasmussen commented Apr 18, 2018

Yup. Works for me! Nice work. 👍 👍

@jorgefilipecosta jorgefilipecosta merged commit 48ce55a into master Apr 18, 2018

2 checks passed

codecov/project 43.94% remains the same compared to 9966a96
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jorgefilipecosta jorgefilipecosta deleted the fix/embed-width branch Apr 18, 2018

@jorgefilipecosta jorgefilipecosta added this to the 2.7 milestone Apr 18, 2018

@@ -142,6 +142,9 @@ class Sandbox extends Component {
width: 100%;
height: 100%;
}
body > div > iframe {

This comment has been minimized.

@aduth

aduth Apr 19, 2018

Member

Isn't the above body.video > div > iframe now redundant with this?

This comment has been minimized.

@jorgefilipecosta

jorgefilipecosta Apr 19, 2018

Member

Hi @aduth ,

Isn't the above body.video > div > iframe now redundant with this?

The part that applies width is, but the rule also sets height for videos and is not applying just to the equivalent selector (body > div > iframe / body.video > div > iframe) but also to other selectors. The equivalent rules without redundancy would be:

			body.video,
			body.video > div,
			body.video > div > iframe {
				height: 100%;
			}

			body.video,
			body.video > div,
			body > div > iframe {
				width: 100%;
			}

or

			body.video,
			body.video > div,
				width: 100%;
				height: 100%;
			}
			body.video > div > iframe {
				height: 100%;
			}
			body > div > iframe {
				width: 100%;
			}

Both alternatives seem worse compared to what we have so preferred to keep selectors for video different from the general ones.

This comment has been minimized.

@aduth

aduth Apr 19, 2018

Member

Oh, right, I missed the other style.

Still, it seems odd that there's overlap, with some styles targeting video specifically, if it's more of a general issue with iframes in embedded content.

nuzzio added a commit to nuzzio/gutenberg that referenced this pull request Apr 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment