search

WordPress / twentytwentytwo

Twenty Twenty-Two, the default WordPress theme that will launch with WordPress 5.9.
406 stars 91 forks source link

Add negative margins to top-level group and cover blocks #336

Closed kjellr closed 2 years ago

kjellr commented 2 years ago

Fixes #330.

I think we overlooked something important in #291: Top level Group blocks (when they have a background color) and Cover blocks should have negative margins applied in both the editor and the front end.

This came up while watching a user struggle with how to make an edge-to-edge header with a solid color background. This should be easy, but it wasn't. Our current rules only apply the negative margins to alignfull blocks, so they had to first add a top-level Group block in the site editor, set it to Inherit default layout, and then place a full-width group w/background color inside of that. It was not intuitive, and is the same hack noted in https://github.com/WordPress/twentytwentytwo/issues/330#issuecomment-1010018513.

Group Blocks with a background color, and Cover blocks already come with have their own padding. So when they're on the top level of the site editor, they don't need to use the outer padding. They can just go edge-to-edge.

Doing this doesn't seem to have any negative effect on our existing layouts or patterns.

To Test:

  1. Add a top-level Group block in the site editor.
  2. Make sure it has left-right padding as expected.
  3. Add a background color to it.
  4. Ensure that the color goes edge-to-edge, and still has inner padding.
  5. Add a top-level cover block, ensure that goes edge-to-edge too.

Also, do a sweep of existing templates and patterns to ensure they're still behaving as intended.

Screenshots

Pattern Preview Before Pattern Preview After
Screen Shot 2022-01-13 at 8 52 24 AM Screen Shot 2022-01-13 at 8 51 59 AM

All blocks in these screenshots are at the top-level of the site editor:

Editor Before Editor After
Screen Shot 2022-01-13 at 8 46 23 AM Screen Shot 2022-01-13 at 8 46 01 AM
Front-end Before Front-end After
Screen Shot 2022-01-13 at 8 46 25 AM Screen Shot 2022-01-13 at 8 45 58 AM
richtabor commented 2 years ago

Just to be clear, the expectation is that contents within an alignfull block are 100% edge-to-edge in the viewport, regardless of parent block padding? (Also may be unrelated to this PR 😬).

CleanShot 2022-01-13 at 09 23 57@2x

richtabor commented 2 years ago

This came up while watching a user struggle with how to make an edge-to-edge header with a solid color background. This should be easy, but it wasn't.

With this PR active, how is this achievable? While editing the header part itself, I can see it's fullwidth, but within the Site Editor, it's not.

CleanShot 2022-01-13 at 09 38 02@2x

Do we perhaps need to target within block template parts as well?

CleanShot 2022-01-13 at 09 36 15@2x

And the frontend of the above:

CleanShot 2022-01-13 at 09 38 55@2x
richtabor commented 2 years ago

I wonder if that particular issue (the header being difficult to make fullwidth) is more related to Gutenberg? As it's not possible to set a template part to fullwidth, or even top-level blocks within it. If we could set that Group block as fullwidth, then the left/right negative margin would be applied properly at the top level. Instead, we are required to double-up Group blocks, just to set the group > group (or group > row) with a background color.

Can't set a top-level block within a template part to fullwidth:

CleanShot 2022-01-13 at 09 40 24@2x

Non top-level blocks can be fullwidth:

CleanShot 2022-01-13 at 09 41 51@2x
jffng commented 2 years ago

With this PR active, how is this achievable? While editing the header part itself, I can see it's fullwidth, but within the Site Editor, it's not.

I think the header part needs to be set to inherit default layout, and the nested group block's alignment set to full.

kjellr commented 2 years ago

Just to be clear, the expectation is that contents within an alignfull block are 100% edge-to-edge in the viewport, regardless of parent block padding? (Also may be unrelated to this PR 😬).

I think that one is unrelated to this PR. But it sounds like a bug. Would you mind sharing the steps to reproduce?

@richtabor: Do we perhaps need to target within block template parts as well?

@jffng: I think the header part needs to be set to inherit default layout

Yeah, but it's not possible to set a template to inherit default layout anymore. So you'd have to use a group block wrapper in there.

In any case, I forgot about template parts initially. 🙃 Pull the new updates and give those a try. Should be better now.

https://user-images.githubusercontent.com/1202812/149362283-7e76a66f-91d9-49a5-8ac2-51c6ea6f0275.mp4

kjellr commented 2 years ago

When we manually add the alignfull class to a block that is not inheriting layout, it will not appear as full width in the preview, but it will appear full width when inserted if it's added at the root because of the theme's alignment CSS

Yeah... We could write in some CSS to fix that, but since alignfull there isn't technically supported, I think it gets a little weird. I'm on the fence about it, but I feel like we could add that in separately from these changes and it would be fine.

kjellr commented 2 years ago

I see. I was testing against the 5.9 RC, not Gutenberg trunk.

Ah yes — last I saw, the change that removes that feature from template parts is going to be back ported to the next RC. 👍

kjellr commented 2 years ago

I've done a bit more testing on my end, and it's still seeming solid. I'm going to merge it in. Thanks folks!