Blockbase: fixed alignments, take 999

[MaggieCabrera]

Changes proposed in this Pull Request:

I tried to understand why we were modifying margin+width for any nested containers but I couldn't see it clearly after reading the original PR that modifies it. I think I might be missing an edge case for this. I spent most of the time in this PR trying to find the ways this could break, I think having test content to cover most of the possible scenarios would help a lot.

This is the code that I used to test against, it is mostly based on the previous issues with have with alignments here and here:

<!-- wp:paragraph -->
<p>This is a paragraph outside of a Group block. It should align on the left and right edges with the paragraph below.</p>
<!-- /wp:paragraph -->

<!-- wp:group {"align":"full","layout":{"inherit":true}} -->
<div class="wp-block-group alignfull"><!-- wp:paragraph -->
<p>This is a paragraph inside of a Group block. It should align on the left and right edges with the paragraph above.</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->

<!-- wp:spacer -->
<div style="height:100px" aria-hidden="true" class="wp-block-spacer"></div>
<!-- /wp:spacer -->

<!-- wp:paragraph -->
<p>This is a query block with full width featured images:</p>
<!-- /wp:paragraph -->

<!-- wp:query {"queryId":15,"query":{"perPage":1,"pages":0,"offset":0,"postType":"post","categoryIds":[],"tagIds":[],"order":"desc","orderBy":"date","author":"","search":"","exclude":[],"sticky":"","inherit":false},"align":"full"} -->
<div class="wp-block-query alignfull"><!-- wp:post-template -->
<!-- wp:group {"layout":{"inherit":true}} -->
<div class="wp-block-group"><!-- wp:post-title {"isLink":true} /-->

<!-- wp:post-featured-image {"isLink":true,"align":"full"} /-->

<!-- wp:post-excerpt /-->

<!-- wp:post-date /--></div>
<!-- /wp:group -->
<!-- /wp:post-template --></div>
<!-- /wp:query -->

<!-- wp:cover {"overlayColor":"primary","align":"full"} -->
<div class="wp-block-cover alignfull has-primary-background-color has-background-dim"><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Full width cover block, needs to touch the browser's borders</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

<!-- wp:group {"backgroundColor":"primary","textColor":"background","layout":{"inherit":true}} -->
<div class="wp-block-group has-background-color has-primary-background-color has-text-color has-background"><!-- wp:paragraph -->
<p>This is a group block with "inherit default layout" turned on.</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->

Before	After

Note that in the Before screenshot you can see a gap to the right of the site that was causing a horizontal scroll. That is also fixed with this PR

Related issue(s):

Closes #4426

[pbking]

I fear there may be more wrong than just that bit... The editor and view are no longer matching in some situations.

View:

Editor:

I think that we've made some incorrect assumptions about "How to identify containers that inherit layout" and how we should be applying that padding (the editor is applying the container- class to ALL containers regardless of if they inherit layout, define layout sizes or not) and this might need a deeper dive considering the "flex" layout in play.

However the mismatch pictured above is wrong in /trunk, not caused by this change, so I believe it to be a separate-but-related issue. I'm keen to bring this change in now. I tried as many nested container situations as I could think of and (other than the problem in the editor noted above) this seems to be a step in the right direction.

I'm definitely keen for others to look at this too though.

[scruffian]

I think the problem with this approach is that nested "inherit layout" containers don't collapse margins.

Take this example markup:

<!-- wp:group {"backgroundColor":"secondary","textColor":"background","layout":{"inherit":true}} -->
<div class="wp-block-group has-background-color has-secondary-background-color has-text-color has-background"><!-- wp:paragraph -->
<p>this is a group</p>
<!-- /wp:paragraph -->

<!-- wp:group {"backgroundColor":"selection","layout":{"inherit":true}} -->
<div class="wp-block-group has-selection-background-color has-background"><!-- wp:paragraph -->
<p>this is a group inside a group</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:group -->

For this case we get this:

So the child "inherit layout" block inherits additional unexpected padding...

[MaggieCabrera]

I fixed the nested containers with the extra padding. Still not happy with this alignments file...

[jffng]

Removing the padding from nested containers has the following effect in my testing — text does not have padding at smaller viewports:

[MaggieCabrera]

I just gave this another spin. Instead of adding padding to every block, I'm trying to add it to the container, then find the exceptions when we don't want that padding to be applied.

I'm trying to think about what we are trying to achieve and what are all the possible cases:

We want our blocks to have padding on mobile so that it's not touching the borders of the viewport, except for alignfull blocks. But when the alignfull block is a group and it contains text, this exception to the rule makes it look bad (like @jffng pointed out on the screenshot above). Should the exception be that only certain blocks that are alignfull should be going to the edges of the viewport then (cover, image, group with a background color...)?

[MaggieCabrera]

And in the case of the alignfull group blocks, if we want padding inside them, we can just add them to the block pattern/template, instead of having an overall css rule, we may not want to have it the same way in every case?

[pbking]

Concepts merged with #4459 which has shipped.
I believe this can be closed.