-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Remove Group block inner container from Grid variation in classic themes #7016
Remove Group block inner container from Grid variation in classic themes #7016
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the test instructions and screenshots @tellthemachines, made review nice and easy 🙇
✅ Code changes match Gutenberg PR
✅ Tests as expected
✅ Applying along side fix from #7001 results in parity between editor and frontend
Before | After | With TT1 Fix |
---|---|---|
Block markup used
<!-- wp:group {"layout":{"type":"grid","columnCount":"4","minimumColumnWidth":null}} -->
<div class="wp-block-group"><!-- wp:paragraph {"backgroundColor":"black"} -->
<p class="has-black-background-color has-background">Grid 1</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph {"backgroundColor":"gray"} -->
<p class="has-gray-background-color has-background">Grid 2</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph {"style":{"layout":{"columnSpan":2,"rowSpan":1}},"backgroundColor":"blue"} -->
<p class="has-blue-background-color has-background">Grid 3</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph {"style":{"layout":{"columnSpan":2,"rowSpan":1}},"backgroundColor":"red"} -->
<p class="has-red-background-color has-background">Grid 4</p>
<!-- /wp:paragraph -->
<!-- wp:paragraph {"style":{"layout":{"columnSpan":2,"rowSpan":1}},"backgroundColor":"orange"} -->
<p class="has-orange-background-color has-background">Grid 5</p>
<!-- /wp:paragraph --></div>
<!-- /wp:group -->
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is testing nicely for me, too!
Thanks for the reviews! Committed to trunk in r58708. |
Trac ticket: https://core.trac.wordpress.org/ticket/61635
Syncs WordPress/gutenberg#49387 to core.
Testing in TT1, a grid that looks like this in the editor:
Should now look like this in the front end:
Instead of looking like this:
The remaining discrepancy between editor and front end is due to a pseudo-element that TT1 inserts into all Group blocks. There's a fix for that open in #7001.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.