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: Correct the margins of embed block content #7164

Merged
merged 2 commits into from Jun 6, 2018

Conversation

Projects
None yet
3 participants
@tofumatt
Member

tofumatt commented Jun 6, 2018

Fixes #7150

Description

Removed the margins around the <figure> element, which gets the content to expand to fill the content box.

How has this been tested?

Tested in browser and things still work and display as they should. Tried with a YouTube and Vimeo embed.

Screenshots (after)

screenshot 2018-06-06 14 33 03

screenshot 2018-06-06 14 32 56

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.

@tofumatt tofumatt requested a review from WordPress/gutenberg-core Jun 6, 2018

@@ -255,7 +255,7 @@ function getEmbedBlockSettings( { title, description, icon, category = 'embed',
return (
<Fragment>
{ controls }
<figure className={ classnames( className, { 'is-video': 'video' === type } ) }>
<figure className={ classnames( className, 'wp-block-embed__figure', { 'is-video': 'video' === type } ) }>

This comment has been minimized.

@tofumatt

tofumatt Jun 6, 2018

Member

I have no clue as to how to name classes in Gutenberg–is there a doc that points me toward what's right?

@jasmussen

I checked out this branch. Turns out that the regression itself was caused by the removal of the wp-block-embed class. Simply adding that back fixes everything. Note that both the editor.scss and style.scss for the embed block both refer to wp-block-embed.

Should we simply rename your wp-block-embed__figure to wp-block-embed and be done with it?

@jasmussen

This comment has been minimized.

Contributor

jasmussen commented Jun 6, 2018

screen shot 2018-06-06 at 09 05 16

👆after renaming wp-block-embed__figure to wp-block-embed. Note that there are also loading specific styles, as well as some for embed alignments. So we need to test those in a fix.

@tofumatt

This comment has been minimized.

Member

tofumatt commented Jun 6, 2018

screenshot 2018-06-06 16 14 18

Makes sense, I've restored the original class. 👍 Above is the screenshot.

@tofumatt tofumatt requested a review from jasmussen Jun 6, 2018

@jasmussen

👍 👍

Nice work!

@tofumatt tofumatt merged commit 7e64e1d into master Jun 6, 2018

2 checks passed

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

@tofumatt tofumatt deleted the fix/7150-embed-block-style-regression branch Jun 6, 2018

@mtias mtias added this to the 3.1 milestone Jun 20, 2018

@mtias mtias added the [Type] Bug label Jun 20, 2018

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