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:46Z

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 Related to a regression in the latest release label Mar 27, 2020

jasmussen requested review from youknowriad and ellatrix March 27, 2020 14:19

jasmussen self-assigned this Mar 27, 2020

jasmussen commented Mar 27, 2020

View reviewed changes

jasmussen mentioned this pull request Mar 27, 2020

Try fix image block #21167

Merged

6 tasks

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

Alternate fix.

1b421e4

jasmussen force-pushed the try/full-wide-regression-fix branch from 9a47aaa to 1b421e4 Compare March 30, 2020 06:29

youknowriad reviewed Mar 30, 2020

View reviewed changes

Address feedback.

6ccb1bf

youknowriad approved these changes Mar 30, 2020

View reviewed changes

jasmussen merged commit 30536e6 into master Mar 30, 2020

jasmussen deleted the try/full-wide-regression-fix branch March 30, 2020 09:43

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix fullwide regression. #21201

Fix fullwide regression. #21201

jasmussen commented Mar 27, 2020

jasmussen Mar 27, 2020

youknowriad Mar 27, 2020

jasmussen Mar 27, 2020

youknowriad Mar 27, 2020

youknowriad Mar 27, 2020

ellatrix Mar 27, 2020

ellatrix Mar 27, 2020

jasmussen Mar 28, 2020

youknowriad Mar 30, 2020

jasmussen Mar 30, 2020

jasmussen commented Mar 27, 2020

github-actions bot commented Mar 27, 2020 •

edited

Loading

jasmussen commented Mar 27, 2020

jasmussen commented Mar 28, 2020

youknowriad Mar 30, 2020

jasmussen Mar 30, 2020

ellatrix commented Mar 30, 2020

Fix fullwide regression. #21201

Fix fullwide regression. #21201

Conversation

jasmussen commented Mar 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasmussen commented Mar 27, 2020

github-actions bot commented Mar 27, 2020 • edited Loading

jasmussen commented Mar 27, 2020

jasmussen commented Mar 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ellatrix commented Mar 30, 2020

github-actions bot commented Mar 27, 2020 •

edited

Loading