Skip to content

Fix reusable block horizontal padding regression.#21312

Merged
jasmussen merged 2 commits intomasterfrom
try/reusable-block-padding-fix
Apr 8, 2020
Merged

Fix reusable block horizontal padding regression.#21312
jasmussen merged 2 commits intomasterfrom
try/reusable-block-padding-fix

Conversation

@jasmussen
Copy link
Copy Markdown
Contributor

https://github.com/WordPress/gutenberg/pull/21099/files#diff-ee2ed3adbe2578628039530c717a9a93R640 introduced a padding attached to the is-root-container. But a reusable block is also a root container.

This PR unsets that.

Before:

Screenshot 2020-04-01 at 09 23 59

After:

Screenshot 2020-04-01 at 09 23 48

@jasmussen jasmussen added the [Type] Regression Related to a regression in the latest release label Apr 1, 2020
@jasmussen jasmussen self-assigned this Apr 1, 2020
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 1, 2020

Size Change: +20 B (0%)

Total Size: 889 kB

Filename Size Change
build/block-library/editor-rtl.css 7.23 kB +11 B (0%)
build/block-library/editor.css 7.23 kB +9 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.4 kB 0 B
build/api-fetch/index.js 3.79 kB 0 B
build/autop/index.js 2.58 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-editor/index.js 102 kB 0 B
build/block-editor/style-rtl.css 10.2 kB 0 B
build/block-editor/style.css 10.2 kB 0 B
build/block-library/index.js 110 kB 0 B
build/block-library/style-rtl.css 7.54 kB 0 B
build/block-library/style.css 7.55 kB 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/components/index.js 195 kB 0 B
build/components/style-rtl.css 16.6 kB 0 B
build/components/style.css 16.5 kB 0 B
build/compose/index.js 6.21 kB 0 B
build/core-data/index.js 10.7 kB 0 B
build/data-controls/index.js 1.04 kB 0 B
build/data/index.js 8.23 kB 0 B
build/date/index.js 5.36 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.05 kB 0 B
build/edit-navigation/index.js 2.75 kB 0 B
build/edit-navigation/style-rtl.css 279 B 0 B
build/edit-navigation/style.css 280 B 0 B
build/edit-post/index.js 92.9 kB 0 B
build/edit-post/style-rtl.css 12.3 kB 0 B
build/edit-post/style.css 12.3 kB 0 B
build/edit-site/index.js 10.1 kB 0 B
build/edit-site/style-rtl.css 5.02 kB 0 B
build/edit-site/style.css 5.02 kB 0 B
build/edit-widgets/index.js 7.17 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.73 kB 0 B
build/editor/editor-styles-rtl.css 399 B 0 B
build/editor/editor-styles.css 400 B 0 B
build/editor/index.js 42.8 kB 0 B
build/editor/style-rtl.css 3.49 kB 0 B
build/editor/style.css 3.49 kB 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/notices/index.js 1.57 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/rich-text/index.js 14.5 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.6 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@MichaelArestad
Copy link
Copy Markdown
Contributor

I'm still seeing top/bottom margin added to the reusable block seen here:

image

It looks like it's caused by one of the wrapping divs preventing neighboring margins from collapsing into each other (I think). I suspect this is the cause:

image

If you uncheck display: flex, the margin collapses.

image

https://github.com/WordPress/gutenberg/pull/21099/files#diff-ee2ed3adbe2578628039530c717a9a93R640 introduced a padding attached to the is-root-container. But a reusable block is also a root container.

This PR unsets that.
@jasmussen jasmussen force-pushed the try/reusable-block-padding-fix branch from fe90811 to abb52be Compare April 8, 2020 06:41
@jasmussen
Copy link
Copy Markdown
Contributor Author

Nice catch, I'll address that in a separate PR, because this PR is only about the horizontal paddings and therefore relatively "safe", but the fix you suggest needs a little bit more scrutiny.

jasmussen added a commit that referenced this pull request Apr 8, 2020
This is a followup and addition to #21312. It's separate because it's not a regression.
@jasmussen
Copy link
Copy Markdown
Contributor Author

I created #21472 to fix the vertical margin separately.

Also pushed a small fix to address a navigation mode border shift.

@MichaelArestad
Copy link
Copy Markdown
Contributor

Thanks @jasmussen. In that case, I think this one is good to go. Nice fix!

@jasmussen
Copy link
Copy Markdown
Contributor Author

Thanks! Can you approve the PR?

@MichaelArestad MichaelArestad self-requested a review April 8, 2020 18:45
@MichaelArestad
Copy link
Copy Markdown
Contributor

Done!

@jasmussen jasmussen merged commit 7aa7e36 into master Apr 8, 2020
@jasmussen jasmussen deleted the try/reusable-block-padding-fix branch April 8, 2020 18:46
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Apr 8, 2020
jasmussen added a commit that referenced this pull request Apr 10, 2020
This is a followup and addition to #21312. It's separate because it's not a regression.
jasmussen added a commit that referenced this pull request Apr 10, 2020
This is a followup and addition to #21312. It's separate because it's not a regression.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Regression Related to a regression in the latest release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants