Fix fullwide regression. #21201

jasmussen · 2020-03-27T14:19:06Z

Full-wide blocks were no longer full-width. This PR fixes that.

This possibly regressed in #21099.

Before:

After:

jasmussen · 2020-03-27T14:19:47Z

packages/block-editor/src/components/block-list/style.scss

+	// Full-wide needs negative margins to accommodate the root container padding.
+	&[data-align="full"],
+	&.alignfull {
+		margin-left: -$block-padding;


We should retire/rename both of the variables used here, as they are no longer descriptive of their original intent.

I kept this style but I moved it to the place where we add the padding to the block wrapper.

Do we really need to add it here? Why?

https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/block-list/style.scss#L650-L662

I don't know... Do you not see the margin issue in master? Is it just a cache ghost ok my machine? Note that I'm using a theme that doesn't provide an editor style.

Yes, it seems to happen only for themes without editor styles, still trying to figure out why

Actually, I figured it out, it depends on the block. Try Media&Text for instance, you'll notice that it works just fine.

I need to run some errands, but I'll have a look after that in a bit. I think somehow we need alignfull/wide on the wrapper div.

I'm not sure I understand the problem... It seems fines even without editor styles

Actually scratch what I said about editor styles. This is also in TwentyTwenty:

It's because of this:

That div wraps the Image block, which means it isn't targetted by this:

.block-editor-block-list__layout.is-root-container > .block-editor-block-list__block[data-align="full"]

I think we need to do two things:

Figure out why the image block has an extra div in the editor — was a new fragment introduced?

Relax or reconsider the idea of scoping the full-wide rule.

What do you think, @youknowriad?

Yes, I haad the same thoughts here

#21201 (comment)

I think we should relax the rule for now and if possible try to find a way to not apply it in inner blocks.

I would agree, but your point about the group block was well made. And if we change the current code (https://github.com/WordPress/gutenberg/pull/21201/files#diff-ee2ed3adbe2578628039530c717a9a93R652) to just relax it overall, that regresses the column block with wide blocks inside.

jasmussen · 2020-03-27T14:20:20Z

This one would be good to get in along with #21097 ;)

github-actions · 2020-03-27T14:24:07Z

Size Change: +58 B (0%)

Total Size: 857 kB

Filename	Size	Change
`build/block-editor/style-rtl.css`	11 kB	+26 B (0%)
`build/block-editor/style.css`	11 kB	+32 B (0%)

ℹ️

View Unchanged

Filename	Size	Change
`build/a11y/index.js`	998 B	0 B
`build/annotations/index.js`	3.45 kB	0 B
`build/api-fetch/index.js`	3.39 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.02 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-library/editor-rtl.css`	7.22 kB	0 B
`build/block-library/editor.css`	7.23 kB	0 B
`build/block-library/index.js`	110 kB	0 B
`build/block-library/style-rtl.css`	7.49 kB	0 B
`build/block-library/style.css`	7.5 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`	190 kB	0 B
`build/components/style-rtl.css`	15.8 kB	0 B
`build/components/style.css`	15.7 kB	0 B
`build/compose/index.js`	6.21 kB	0 B
`build/core-data/index.js`	10.6 kB	0 B
`build/data-controls/index.js`	1.04 kB	0 B
`build/data/index.js`	8.25 kB	0 B
`build/date/index.js`	5.37 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom-ready/index.js`	568 B	0 B
`build/dom/index.js`	3.06 kB	0 B
`build/edit-post/index.js`	91 kB	0 B
`build/edit-post/style-rtl.css`	8.43 kB	0 B
`build/edit-post/style.css`	8.43 kB	0 B
`build/edit-site/index.js`	6.73 kB	0 B
`build/edit-site/style-rtl.css`	2.91 kB	0 B
`build/edit-site/style.css`	2.9 kB	0 B
`build/edit-widgets/index.js`	4.43 kB	0 B
`build/edit-widgets/style-rtl.css`	2.57 kB	0 B
`build/edit-widgets/style.css`	2.57 kB	0 B
`build/editor/editor-styles-rtl.css`	423 B	0 B
`build/editor/editor-styles.css`	426 B	0 B
`build/editor/index.js`	42.8 kB	0 B
`build/editor/style-rtl.css`	3.38 kB	0 B
`build/editor/style.css`	3.38 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`	781 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.55 kB	0 B
`build/shortcode/index.js`	1.7 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.61 kB	0 B
`build/warning/index.js`	1.14 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

jasmussen · 2020-03-27T18:00:55Z

I don't think this is world ending and in need of an urgent patch. If it only happens if you use a theme that doesn't style the editor but does support full width, it's already edge case. I think it's okay if this needs more time in the oven.

jasmussen · 2020-03-28T13:21:18Z

Alright, I have created a different fix based on the learning in #21201 (comment).

The Image block has an alignment wrapper that outputs a div around the image block with a specific class, if necessary. It looks like this:

				<div
					className={
						// Ideally these classes are not needed, and ideally, we
						// provide an alignment wrapper component that the block
						// can wrap around the block or we build it into
						// Block.*.
						needsAlignmentWrapper
							? 'wp-block block-editor-block-list__block'
							: undefined
					}
				>

When the image block has no alignment, or it has full-wide, then that div is just an empty wrapper.

In leiu of refactoring that div away, I created a fix that targets full-wide children of empty divs as well. It's not the prettiest solution in the world, but it's a very simple fix that would be good to ship as we look for better solutions. What do you think, @ellatrix?

youknowriad · 2020-03-30T08:24:36Z

packages/block-editor/src/components/block-list/style.scss

@@ -649,6 +649,9 @@

 .block-editor-block-list__layout.is-root-container {
 	// Full-wide. (to account for the padddings added above)
+	// The first two rules account for the alignment wrapper div for the image block.
+	> div > .block-editor-block-list__block[data-align="full"],
+	> div > .block-editor-block-list__block.alignfull,


maybe we can add > div:not(.block-editor-block-list__block) instead of > div because that markup is definitely possible in nested contexts

I suppose we're already in "very very specific selectors" territory — but the :not rule would make it even more specific. But I'm happy to add it.

ellatrix · 2020-03-30T10:11:33Z

Maybe I should have included this in the patch release 😅

jasmussen added the [Type] Regression label Mar 27, 2020

jasmussen requested review from youknowriad and ellatrix Mar 27, 2020

jasmussen self-assigned this Mar 27, 2020

jasmussen reviewed Mar 27, 2020

View changes

jasmussen mentioned this pull request Mar 27, 2020

Try fix image block #21167

Merged

6 tasks done

jasmussen force-pushed the try/full-wide-regression-fix branch 2 times, most recently from 0d11df6 to 9a47aaa Mar 28, 2020

Alternate fix.

1b421e4

jasmussen force-pushed the try/full-wide-regression-fix branch from 9a47aaa to 1b421e4 Mar 30, 2020

youknowriad reviewed Mar 30, 2020

View changes

Address feedback.

6ccb1bf

youknowriad approved these changes Mar 30, 2020

View changes

jasmussen merged commit 30536e6 into master Mar 30, 2020
3 checks passed

3 checks passed

build
Details

pull-request-automation
Details

Travis CI - Pull Request Build Passed
Details

jasmussen deleted the try/full-wide-regression-fix branch Mar 30, 2020

github-actions bot added this to the Gutenberg 7.9 milestone Mar 30, 2020

WordPress / gutenberg

Fix fullwide regression. #21201

Fix fullwide regression. #21201

jasmussen commented Mar 27, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jasmussen commented Mar 27, 2020

github-actions bot commented Mar 27, 2020 •

edited

jasmussen commented Mar 27, 2020

jasmussen commented Mar 28, 2020

This comment has been minimized.

This comment has been minimized.

ellatrix commented Mar 30, 2020

WordPress / gutenberg

Fix fullwide regression. #21201

Fix fullwide regression. #21201

Conversation

jasmussen commented Mar 27, 2020

This comment has been minimized.

jasmussen Mar 27, 2020 Author Contributor

This comment has been minimized.

youknowriad Mar 27, 2020 Contributor

This comment has been minimized.

jasmussen Mar 27, 2020 Author Contributor

This comment has been minimized.

youknowriad Mar 27, 2020 Contributor

This comment has been minimized.

youknowriad Mar 27, 2020 Contributor

This comment has been minimized.

ellatrix Mar 27, 2020 Member

This comment has been minimized.

ellatrix Mar 27, 2020 Member

This comment has been minimized.

jasmussen Mar 28, 2020 Author Contributor

This comment has been minimized.

youknowriad Mar 30, 2020 Contributor

This comment has been minimized.

jasmussen Mar 30, 2020 Author Contributor

jasmussen commented Mar 27, 2020

github-actions bot commented Mar 27, 2020 • edited

jasmussen commented Mar 27, 2020

jasmussen commented Mar 28, 2020

This comment has been minimized.

youknowriad Mar 30, 2020 Contributor

This comment has been minimized.

jasmussen Mar 30, 2020 Author Contributor

ellatrix commented Mar 30, 2020

jasmussen Mar 27, 2020
Author Contributor

youknowriad Mar 27, 2020
Contributor

jasmussen Mar 27, 2020
Author Contributor

youknowriad Mar 27, 2020
Contributor

youknowriad Mar 27, 2020
Contributor

ellatrix Mar 27, 2020
Member

ellatrix Mar 27, 2020
Member

jasmussen Mar 28, 2020
Author Contributor

youknowriad Mar 30, 2020
Contributor

jasmussen Mar 30, 2020
Author Contributor

github-actions bot commented Mar 27, 2020 •

edited

youknowriad Mar 30, 2020
Contributor

jasmussen Mar 30, 2020
Author Contributor