Try: add padding CSS rules instead of in the templates

[MaggieCabrera]

This PR provides an interim solution to the padding problem that eventually will be solved in Gutenberg (WordPress/gutenberg#35607). This leverages a custom outer spacing variable that will be absorbed with the future variable implemented for the editor plus a series of CSS rules that will eventually disappear as the editor provides a global solution instead.

To test this check that the full width blocks all over the theme go all the way to the borders of the viewport while the rest of the site maintains a padding on mobile to stop content from extending. The editor should show the same as the frontend does.

Alternative to #289

Addresses #234

[kjellr]

In general, this is looking promising. I know this is in progress, but leaving a few notes here:

Many of our full-width patterns provide their own padding to ensure their inner content doesn't bump up against the edges. But currently, they are bumping up against the edges even so:

Also, I think we need to be more specific about these rules. Here's an example of the post content placed inside of a column. As you can see, the left/right negative margin is being applied when it shouldn't be:

[MaggieCabrera]

Thanks @kjellr! Generally, I've found in most cases the only problem is that we are applying full width where we really don't need to do so. Such is the case of the first pattern that you mention (I've just fixed that). I'll have a look at the other case, but I think we could use as many eyes on this as possible so we cover all the possible cases.

I'll clean up this PR so that it's ready for review.

[kjellr]

we are applying full width where we really don't need to do so

Could be. But we also need to ensure there are fallbacks in place because we can't control what weird stuff other people do. This is exactly why the layout rules tend to get so complicated. 😅

[MaggieCabrera]

@kjellr how did you manage to get that full width item inside the two column layout? it doesn't let me:

[kjellr]

I set the Query block there to inherit the default layout. Generally that's a bad idea in this setup, but I was trying to break it and find holes. 😄 When I did this, it looked fine in the editor, but broke in the front end.

[kjellr]

I pushed a few small fixes to templates. In general, this is looking pretty good — I've been running through a bunch of content and haven't found any major errors. We should try some tests where there are alignfull items inside of columns though, cause I expect there could be issues.

I'm going to mark it as ready for a true review.

[MaggieCabrera]

I introduced the exception for columns, I think this is the intended behavior, right?

This is the markup I sued on the example above:

<!-- wp:columns {"align":"full"} -->
<div class="wp-block-columns alignfull"><!-- wp:column {"backgroundColor":"primary","textColor":"background"} -->
<div class="wp-block-column has-background-color has-primary-background-color has-text-color has-background"><!-- wp:paragraph -->
<p>1 column full aligned</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->
<!-- wp:columns -->
<div class="wp-block-columns"><!-- wp:column {"backgroundColor":"secondary"} -->
<div class="wp-block-column has-secondary-background-color has-background"><!-- wp:paragraph -->
<p>column 1</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->

<!-- wp:column {"backgroundColor":"secondary"} -->
<div class="wp-block-column has-secondary-background-color has-background"><!-- wp:paragraph -->
<p>Column 2</p>
<!-- /wp:paragraph -->

<!-- wp:group {"layout":{"inherit":true}} -->
<div class="wp-block-group"><!-- wp:group {"align":"full","backgroundColor":"primary","textColor":"background"} -->
<div class="wp-block-group alignfull has-background-color has-primary-background-color has-text-color has-background"><!-- wp:paragraph -->
<p>group full aligned</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:group -->

<!-- wp:paragraph -->
<p>---</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

<!-- wp:columns {"align":"full"} -->
<div class="wp-block-columns alignfull"><!-- wp:column {"backgroundColor":"secondary"} -->
<div class="wp-block-column has-secondary-background-color has-background"><!-- wp:paragraph -->
<p>column 1</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column -->

<!-- wp:column {"backgroundColor":"secondary"} -->
<div class="wp-block-column has-secondary-background-color has-background"><!-- wp:paragraph -->
<p>Column 2</p>
<!-- /wp:paragraph -->

<!-- wp:group {"layout":{"inherit":true}} -->
<div class="wp-block-group"><!-- wp:group {"align":"full","backgroundColor":"primary","textColor":"background"} -->
<div class="wp-block-group alignfull has-background-color has-primary-background-color has-text-color has-background"><!-- wp:paragraph -->
<p>group full aligned</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group --></div>
<!-- /wp:group -->

<!-- wp:paragraph -->
<p>---</p>
<!-- /wp:paragraph --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

[kjellr]

That looks about right to me.

---

A couple general questions I thought of this morning:

• Is there anything to this approach/CSS that could be ported upstream to Gutenberg instead? At a glance, this code isn't very specific to this theme. If we merge this, I expect other themes will copy/paste this code, and if possible it might be better to just place it in Gutenberg somewhere so it can be revised and replaced in the future more easily.
• We should test alongside the current implementation of sit padding, and make sure it doesn't break horribly.

[MaggieCabrera]

Ugh, I just noticed the 1 column example above is not correct, I'll have another pass at that.

[MaggieCabrera]

* Is there anything to this approach/CSS that could be ported upstream to Gutenberg instead? At a glance, this code isn't very specific to this theme. If we merge this, I expect other themes will copy/paste this code, and if possible it might be better to just place it in Gutenberg somewhere so it can be revised and replaced in the future more easily.

I don't think so, no. It needs more testing, to see if we've missed something somewhere we don't want the negative margins besides the columns case. The idea for GB was to be able to handle the padding variable with a built-in variable so themes could opt-in into this, else you will break themes that implement this differently or don't want this behavior at all.

* We should test alongside the current implementation of sit padding, and make sure it doesn't break horribly.

I don't know what you mean exactly, what implementation?

[MaggieCabrera]

@kjellr for the case of a full width block inside a one column that is not full width, the current behavior doesn't make the inner block go all the way to the borders because of the fixed width of the parent so I don't think we need a rule for those. I'm removing that exception

[MaggieCabrera]

The group thing, on trunk:

On this PR:

[MaggieCabrera]

This is ready for another look, I don't have any broken cases in my arsenal, please try and break this

[kjellr]

Are you seeing extra padding in the site editor?

Front End	Site Editor

[MaggieCabrera]

Are you seeing extra padding in the site editor?

What I'm seeing is that the alignfull on that block is only working because the block has the class (in the front), but that block doesn't have alignment options showing up on the toolbar, so the editor doesn't add the data attribute that we are targeting in the editor. So I think that's a case of refactoring the template part, do you see it somewhere else?

[kjellr]

Ok I see, we have to add an additional group wrapper there. 😅

Pushed 6210863 to take care of that.

[jffng]

Still forming opinions on whether I think it's a generally a good idea to introduce these global rules to the theme, but right now I am noticing some regressions:

The home page header is getting some Gutenberg padding rules applying to it now, note the .wp-group-block:where rule:

The default footer lacks padding on smaller viewports:

The default header appears at default instead of wide width:

[jffng]

I made three changes to try and resolve the issues reported above:

• Removed a wrapper in the default footer pattern to get the global padding to be respected 5f6c14a
• Removed a wrapper in the default header and also removed inherit: true on the header template part inside the templates. I'm not sure why / how this was got in there to begin with, it also was inconsistent because the 404 and archive templates did not have it set on the header template part: 705a6da
• Added an extra selector to get the default padding of group block's that have a background to match the custom variable we have set. I'm not sure about this one. 395e093

[kjellr]

I found a use case that doesn't work right: A full-width Group block with a background color. The full-width child here is supposed to go to the edge, but it doesn't. I'll try and push a fix.

[kjellr]

I fixed that, and also synced things up so that wide/full blocks inside of a full-width group block are aligned with wide/full blocks outside of one:

Before	After

At the moment, the only issue I'm seeing is that full-width items inside of non-full width parents incorrectly inherit the negative margins and padding:

As far as I can tell, we can't really clear that without breaking all kinds of other stuff. So I think we just have to let it be.

[jffng]

In the site editor, check out the archive or home template. This rule is applying an unwanted padding in the query:

[kjellr]

Yeah, I believe is-root-container is used in multiple places. We should switch to something else or be more specific.

[kjellr]

I just pushed a potential fix in c4a80d3. It uses more specific selectors, but also required specifying both the root container for the site editor and for the post/page editor.

[kjellr]

Hmm actually, now that I'm looking closer I'm not sure that'll do it. 😕

[kjellr]

😕 It doesn't. Reverting that commit.

[kjellr]

Ok, after some further digging, I can see that this is basically a re-creation of this issue: WordPress/gutenberg#33248.

Basically, the Query block renders using some nested DOM elements, and that causes this duplicate selector where it shouldn't be. In the post/page editor, Gutenberg itself provides some is-root-container styles that cause the same behavior you're seeing here.

Since WordPress/gutenberg#33248 is listed as a must-have for 5.9, and is currently in progress I think we're ok to push through with this bug present for now.

[jasmussen]

TT2 is my favorite theme in years, in part because it's able to do so much with just blocks alone. It's clearly shown the need to solve this at the block editor level. It's also shown how tricky it is to get that right, and so I think it's good to buy some time with this solution. Hopefully the usage and rules can help us zero in on the right heuristic to implement.

As for the specific rules, it's definitely a little hard to parse if you haven't built a ton of the CSS. At a high level, the code looks good for me, and in testing, things look right to me:

I also tried some patterns out, and it all appears to work well. The GIF was too big, so it's here.

I would love for the clamp rules to be absorbed by the system as well — the inability to see or intuitively edit the some of the paddings feels a little janky. But that's a separate issue, and probably not one that needs solving in the near future.

Ultimately, if we go with this approach, which as noted I think is good to do for the time being, it seems okay to merge soon and then keep an eye out for anything that might need following up on.

[kjellr]

Thanks for the testing! I also tested with the current Site Padding implementation, and it works fine... it just adds padding to the outside of all this:

Example	Example

---

I'd like to reiterate what I said over in this other thread:

This is not our ideal solution, but it's the least complicated one for today. It's a handful of CSS rules, and it generally works without the user changing their behavior or opting into anything. The end goal is to get a solution merged into Gutenberg, but I think this is a first step towards that and is worth testing during the upcoming Beta cycle.

[jffng]

I think let's bring this since we've noted it as being a temporary solution, and have some consensus that this the best tradeoff to achieve the intended design result with the current tools available.

Kudos @MaggieCabrera for the PR and @jasmussen for all the testing and feedback!

[MaggieCabrera]

Thank you all for testing this, it's a complicated problem!

[richtabor]

Hey all 👋 — love this. I noticed that there's a discrepancy between the use of the spacing.outer var and the padding set by various patterns. If you modify the outer value in theme.json, you'll see that the patterns don't adjust as well (as many set their own padding left/right values.

I took a stab at it in #310 — which lets the patterns adapt to the outer value. Let me know what you think :)