More consistent root padding #60715
More consistent root padding #60715
Conversation
|
Size Change: -75 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in e2a5570. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9217278261
|
richtabor
changed the title
layout-test.mp4Tested this today. Keep in mind this is a difficult one to context switch to so I could be off base in my observations.
I'm guessing a lot of full width patterns will be designed with the toggle off but will still want global padding set. e.g. a full width grid. It's confusing to me why we are treating these differently. If a [group block with inner block toggle enabled] is nested within a [full width group block that has inner block toggle disabled], should it have padding applied? In the old version it does, the new version doesn't. 
Can we simplify and just say any full width group should have global padding applied by default? These changes are an improvement but its still difficult to get a grasp for how global padding works. I understand why we do what we do but it does add a learning curve to WordPress' layout system vs tools like Figma/Framer/Webflow. What can we do to remove some of the "magic"? The Style Inheritance work aims to always show values within the inspector, even if they come from a global context. This will help with the whole unpredictable feeling of global padding. It's important we also keep track of this which is part of broader efforts to move closer to a Figma like approach when it comes to layout and dimension controls. |
richtabor
force-pushed
the
try/root-padding-explorations
branch
from
0f8c928
to
4948a58
Compare
Yes, this is a big part of what I'm experimenting with here. Figure out what's expected vs. not.
Correct, with "content width" enabled, padding is applied to blocks within it. There is no change on desktop, only on mobile (where the padding is arguably missing). Fullwidth blocks are always viewport-to-viewport fullwidth. Trunk / Proposed:
Not quite, here's where we were a bit smart with what's in trunk. In the old version, elements (p, p,h1,h2,h3,h4,h5,h6,ul,ol) have outer padding added back exclusively, when within a fullwidth block. We can likely add this back, but exploration wise it was good to pull it out first. Trunk / Proposed:
I like that idea. I wanted to try and get more stability around the concepts we have today, but I'd like to explore that as well. Essentially assuming outer padding applied to blocks within fullwidth blocks. |
@SaxonF I updated the pr to try this out. It does feel nice not having the "Inner blocks content width" control feel like it's doing too much. It just makes inner blocks fill the available space, or abide by their own width controls. We'd need to do a before/after to explore the delta with this. full-w-padd.mp4 |
|
I prefer that approach because with the toggle disabled layout behaviour is much more like design tools that people will be familiar with where the "magic" is removed. We are just setting a default padding which makes these full width patterns transferrable. When content is constrained/boxed we can be a little smarter with padding and width because its treated almost like a writing a flow. We can be a bit more opinionated as to what works best for long form content (e.g. "full width" groups are 100vw and bypass padding) |
There was a problem hiding this comment.
Thanks for trying this out!
Fullwidth blocks are always viewport-to-viewport fullwidth.
Just noting that full width blocks are the width of their container, not necessarily the viewport.
I'm seeing a few differences between editor and front end in this branch, so which is the intended approach? The editor generally looks good with the combinations I've tested, but the front end is very flaky, with padding applied to flex blocks and negative margins on children of blocks that have no padding:
Front end: the columns with coloured backgrounds were supposed to be full width
Editor is looking better:
|
@tellthemachines do you mind sharing the block markup you're using to test. Want to make sure we're 1:1. |
|
Sure! I tested with a columns block containing a fixed width column, and another taking up all the remaining space: MarkupI also created a pattern with a very similar layout except using Rows and Stacks instead of Columns: MarkupI tested both of these by placing them at the root of a template. |
richtabor
force-pushed
the
try/root-padding-explorations
branch
from
6a00841
to
16769a5
Compare
|
@tellthemachines would you mind giving this another run? I missed a conditional.
I'll do an audit of before/afters tomorrow to get a clearer picture, but I do know that this would change alignfull blocks with contents to have global padding. |
There was a problem hiding this comment.
Thanks for updating @richtabor! This is looking much better and front end now matches editor.
The one difference I'm finding with trunk that I don't believe is intended is the root padding class now being applied on all full width flex and grid blocks. This means that it's no longer possible to have e.g. a Gallery block going full width:
|
How are we looking with this one? Would love to see it get merged at some point (and in 6.6) and would be happy to test along the way. |
richtabor
force-pushed
the
try/root-padding-explorations
branch
from
7b88696
to
cd2f2d5
Compare
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/class-wp-theme-json-gutenberg.php ❔ phpunit/class-wp-theme-json-test.php |
I'm taking another look at it today, mind running it in a bit? |
|
@richtabor Yes, of course. Just LMK when it's ready and I'll throw it up via Playground. |
richtabor
changed the title
richtabor
added
[Feature] Layout
Twenty Twenty FourDemonstrating that there are no changes to existing patterns: Patterns
Template Parts / Focus View
No changes on front-end:
|
tellthemachines
force-pushed
the
try/root-padding-explorations
branch
from
28ace51
to
4d4712e
Compare
|
Changeset committed to core in https://core.trac.wordpress.org/changeset/58226. All done now! (except the JS changes which should go in with the next package update) |
fabiankaegy
added
the
Needs Dev Note
Unlinked contributors: bridgetwes. Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: SaxonF <saxonafletcher@git.wordpress.org> Co-authored-by: bgardner <bgardner@git.wordpress.org> Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org> Co-authored-by: mikemcalister <mmcalister@git.wordpress.org>
|
Hey @tellthemachines @richtabor Great to see this cleanup done! However, the newly added
The content block is no longer aligning as expected, resulting instead in triple padding being applied to the sides of the content block! The base global padding, and then an extra one applied to each group wrapper block 😭 I can manually set the side padding for those group blocks to zero, but I'm mentioning it just in case this was an unintentional consequence that you'd want to take another look at 😊 P.S. I created a PR #62470 to fix the |
|
Oh actually, TIL that |
Unlinked contributors: bridgetwes. Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: SaxonF <saxonafletcher@git.wordpress.org> Co-authored-by: bgardner <bgardner@git.wordpress.org> Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org> Co-authored-by: mikemcalister <mmcalister@git.wordpress.org>
|
@juanmaguitar here's a dev note for this PR: Root padding style updatesRoot padding styles have been updated to address inconsistencies in pattern display and make the application of padding more predictable across different sets of markup. It’s now expected that:
|
Closes #60308.
I'm using this gist as a basis for testing. The content should have the same layout behavior throughout WordPress.
Why
Patterns today, even in Twenty Twenty Four, have to hard-code left and right padding values into theme patterns, so that patterns render properly with left and right spacing anywhere they were used.
People creating patterns on their sites are required to do the same, even though it's not clear. In some instances when creating a pattern the outer padding is expressed as expected, but when you view that pattern in a focus view, it's not.
Here's why that's not ideal:
Testing Instructions
Then do the same testing, but with these two sets of block markup:
Key Results
Fullwidth:
Application of outer padding: