Automattic / themes

Free WordPress themes made by Automattic for WordPress.org and WordPress.com.
https://themeshaper.com
GNU General Public License v2.0
873 stars 351 forks source link

[Twenty Twenty-One] Grid Block stacks to a single column in editor #7943

Closed mrfoxtalbot closed 3 weeks ago

mrfoxtalbot commented 1 month ago

Quick summary

The grid block does not show multiple columns when used with Twenty Twenty-One on a dotcom site:

Screenshot 2024-07-09 at 15 20 05

Below is what the Grid Block looks like in the editor using TT1 on a self-hosted site:

Screenshot 2024-07-09 at 12 27 14

The layout is also broken in the front end (in the published post) but this second issue appears to be coming directly from core and was reported here: https://github.com/WordPress/gutenberg/issues/63300

Screenshot 2024-07-09 at 12 30 47

Steps to reproduce

  1. Activate Twenty Twenty-One
  2. Create a new post and insert a Grid block
  3. Notice how it stacks to as single column

What you expected to happen

It should show a grid

What actually happened

It stacked into a single column.

Impact

All

Available workarounds?

No but the platform is still usable

Platform (Simple and/or Atomic)

Simple, Atomic

Logs or notes

No response

mrfoxtalbot commented 1 month ago

@dsas, sorry to keep pinging you but could you take a look to help triage this? Please note that the Grid block is also partially broken in core https://github.com/WordPress/gutenberg/issues/63300 Thank you!

up1512001 commented 1 month ago

This is an issue with the TT1 theme I checked it's due to this code present in the style.css file at the 2154 line number.

.wp-block-group:before,
.wp-block-group:after {
    content: "";
    display: block;
    clear: both;
}

before removing this code

Screenshot 2024-07-09 at 23 35 24

after removing this code

Screenshot 2024-07-09 at 23 35 50
dsas commented 1 month ago

@up1512001 fix looks like it will probably fix the problem but I don't think it's the correct fix. @mrfoxtalbot is right that this problem doesn't exist in dotorg. It looks to me like the root cause is that the markup output differs between dotorg and dotcom.

dotcom markup

(excess newlines removed)

<div class="wp-block-group is-layout-grid wp-container-core-group-is-layout-1 wp-block-group-is-layout-grid">
<p>First cell</p>
<p>Second cell</p>
<p>Third cell</p>
</div>

dotorg markup

(excess newlines removed)

<div class="wp-block-group"><div class="wp-block-group__inner-container is-layout-grid wp-container-core-group-is-layout-1 wp-block-group-is-layout-grid">
<p>First cell</p>
<p>Second cell</p>
<p>Third cell</p>
</div></div>

The way this works in TT1 on a dotorg site is that the outer div - div.wp-block-group has the display:block rule but the inner div - div.wp-block-group__inner-container has a display:grid.

For some reason the dotcom output doesn't include the inner div. It looks like it's been added to GB for a long time so I'm not sure why it is not being output yet.

dsas commented 1 month ago

Sorry @up1512001 after having done some research it appears that this was intentional, and so we do just need to fix the styles. More detail over on the GB issue.

lsl commented 1 month ago

@dsas This was also being tracked on the one board, if you'd like help with reviews or anything let us know.

mrfoxtalbot commented 1 month ago

I see a commit in trunk that should fix this. Check this comment. https://github.com/WordPress/gutenberg/issues/63300#issuecomment-2224751292

lsl commented 1 month ago

I think that might just make core behave the same way, we'll need to fix the css in 2021 theme.

Thanks for the ping! This made me realise that https://github.com/WordPress/gutenberg/pull/49387 was never synced to core when the grid layout was stabilised, which it should have! So the broken behaviour you're seeing with Gutenberg enabled would also be happening in core if that PR had been synced. ... Quick follow-up on this: the sync of https://github.com/WordPress/gutenberg/pull/49387 has now been committed to core trunk.

dsas commented 3 weeks ago

There is now a commit in trunk to fix this.

Looks like our copy of TT1 is rather out of date. I'll look at merging it, following this process tomorrow: PNEWy-cvG-p2

dsas commented 3 weeks ago

Merged from core and shipped on wpcom in r66773. Big thanks to @up1512001 for the upstream work.