Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grid Block layout issues with Twenty Twenty-One #63300

Closed
mrfoxtalbot opened this issue Jul 9, 2024 · 7 comments
Closed

Grid Block layout issues with Twenty Twenty-One #63300

mrfoxtalbot opened this issue Jul 9, 2024 · 7 comments
Labels
[Block] Group Affects the Group Block (and row, stack and grid variants) [Type] Bug An existing feature does not function as intended

Comments

@mrfoxtalbot
Copy link

Description

The Grid block has some layout issues when used with Twenty Twenty-One.

The editor looks good:

Screenshot 2024-07-09 at 12 27 14

...but the layout in the published post is broken:

Screenshot 2024-07-09 at 12 30 47

Depending on the details, if the issue is coming from the theme, this would need to be moved to Trac (it is a bundled theme).

Step-by-step reproduction instructions

  1. Add a Grid (group) block to TT1
  2. Check the layout in the published post

Screenshots, screen recording, code snippet

No response

Environment info

  • WP 6.6 RC2

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@mrfoxtalbot mrfoxtalbot added [Type] Bug An existing feature does not function as intended [Block] Group Affects the Group Block (and row, stack and grid variants) labels Jul 9, 2024
@up1512001
Copy link
Member

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

@sabernhardt
Copy link
Contributor

@dsas
Copy link
Contributor

dsas commented Jul 10, 2024

I think that the PR @up1512001 opened is the right approach.

I can only reproduce this when using the GB plugin - 18.7.1 (testing with 6.6-RC3-58701) and trunk (testing with 6.5.5). I can't reproduce it on WP6.5 or WP6.6 alone.

This is happening because of the TT1 styles that @up1512001 mentions, in combination with a difference in html output when GB is enabled vs disabled - which is why this isn't always reproducible.

GB enabled 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>

Vanilla 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 vanilla 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 rule.

The problem doesn't exist if I remove the grid check in this line if ( isset( $block['attrs']['layout']['type'] ) && ( 'flex' === $block['attrs']['layout']['type'] || 'grid' === $block['attrs']['layout']['type'] ) ) from gutenberg_restore_group_inner_container. This tallys with the core wp version of this function which is used when GB is enabled as that doesn't return early for grid blocks either.

This early return was added by @tellthemachines in #49387 and it was judged " it's up to themes to update their styles if they want to support new features, so I don't think it's a huge issue"

@tellthemachines
Copy link
Contributor

Thanks for the ping! This made me realise that #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.

I agree a code update in TT1 is the best way to fix it, but not 100% sure how - might be good to get someone who knows the theme well to review the proposed fix. Maybe @karmatosed ?

I also notice that in 6.6, due to the inner wrapper not being removed, some grid children aren't stretching the full width of their grid cell because of conflicting styles applied to the inner wrapper. So we get this:

Screenshot 2024-07-12 at 9 52 18 AM

Where really the blocks should look like this:

Screenshot 2024-07-12 at 9 53 58 AM

(ignore the different background colors, I was testing customizer styles which don't apply in the editor 😅 )

I'll get a core sync PR for #49387 ready. Might be good to try and get it into 6.6.1, cc @ellatrix

@tellthemachines
Copy link
Contributor

Quick follow-up on this: the sync of #49387 has now been committed to core trunk.

@dsas
Copy link
Contributor

dsas commented Aug 5, 2024

The upstream TT1 changes to fix the issue have been merged

@mrfoxtalbot
Copy link
Author

I re-tested this and I can confirm it is fixed. Thank you all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Group Affects the Group Block (and row, stack and grid variants) [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

5 participants