Try using variable for root padding value #39926
Conversation
tellthemachines
requested review from
TimothyBJacobs,
spacedmonkey and
ellatrix
as code owners
|
Size Change: +5.46 kB (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
| '--wp--style--padding-top' => array( 'spacing', 'padding', 'top' ), | ||
| '--wp--style--padding-right' => array( 'spacing', 'padding', 'right' ), | ||
| '--wp--style--padding-bottom' => array( 'spacing', 'padding', 'bottom' ), | ||
| '--wp--style--padding-left' => array( 'spacing', 'padding', 'left' ), |
There was a problem hiding this comment.
I'm not sure why this only seems to affect the root styles 🤔
| "right": "17px", | ||
| "bottom": "17px", | ||
| "left": "17px" | ||
| } |
There was a problem hiding this comment.
The "padding" object is the only real addition to this file.
There was a problem hiding this comment.
Is this like a demo padding that applies to all themes? won't this be a breaking change for existing themes?
There was a problem hiding this comment.
Yeah, I guess it could be if the theme doesn't define any root padding. I was mainly thinking of a fix for #35884, but perhaps that should be post editor specific.
There was a problem hiding this comment.
Yeah this is problem for themes that aren't opting in to site padding. I think we'll need to add it programatically.
| @@ -319,6 +319,11 @@ | |||
| } | |||
| } | |||
|
|
|||
| .is-root-container > .alignfull { | |||
| margin-right: calc(var(--wp--style--padding-right) * -1); | |||
| margin-left: calc(var(--wp--style--padding-left) * -1); | |||
There was a problem hiding this comment.
Not sure if we should have a fallback value here in case the variable doesn't exist.
packages/blocks/src/api/constants.js
Outdated
| '--wp--style--padding-top': { | ||
| value: [ 'spacing', 'padding', 'top' ], | ||
| support: [ 'spacing', 'padding' ], | ||
| rootOnly: true, |
There was a problem hiding this comment.
This feels super hacky, but without it these styles get applied to all blocks that have padding set on them. Any ideas welcome!
There was a problem hiding this comment.
Should global styles also output these vars for specific blocks?
I bumped the padding value of the Group block in global styles and I see this in the global-styles-inline-css style tag:
body {
...
--wp--style--padding-top: 173px;
--wp--style--padding-right: 173px;
--wp--style--padding-bottom: 173px;
--wp--style--padding-left: 173px;
--wp--style--block-gap: 1.5rem;
}
...
.wp-block-group {
--wp--style--padding-top: 142px;
--wp--style--padding-right: 142px;
--wp--style--padding-bottom: 142px;
--wp--style--padding-left: 142px;
}The editor reflects the changes but the frontend doesn't, because, I guess, we'd need to either update the blocks' CSS to use these vars or print it on the fly somewhere in the backend.
On a related note, gap also gets special treatment in WP_Theme_JSON_Gutenberg via 'blockGap' => 'top',, the effect of which is that --wp--style--block-gap is only rendered at the "top" (or root) level in global styles. Effectively the body tag.
But we also want block-level global styles, when set, to override any root value. It's something we're trying resolve over at #39870
It seems like we do need some sort of flag that says "Hey, this property only lives at the root level, so output its style rule in the body tag and nowhere else".
I'm not sure what that would look like yet, maybe a const VALID_TOP_LEVEL_STYLES in WP_Theme_JSON_Gutenberg that explicitly defines root-level-only styles 🤷
There was a problem hiding this comment.
Should global styles also output these vars for specific blocks?
Oooooh no it shouldn't! It's actually outputting the variables instead of the properties in the front end now, and that's messing with some of the negative margin calculations 😫
It seems like we do need some sort of flag that says "Hey, this property only lives at the root level, so output its style rule in the body tag and nowhere else".
Yup, we definitely need something like this. I was playing around with it on the editor side here, not realising it would also be needed on the server side.
I'm not sure what that would look like yet, maybe a const VALID_TOP_LEVEL_STYLES in WP_Theme_JSON_Gutenberg that explicitly defines root-level-only styles
Can we make do with the "top" value in VALID_STYLES solution we have currently? What shortcomings do you see with it?
There was a problem hiding this comment.
Ok I made some changes that should fix the block-level global styles issue.
The thing with padding is that we only need it to be a variable at the root level, so we can easily apply it to all the places we need it: as padding on full-width root level blocks, or as negative margin on their full-width children. I'm not sure if block gap works exactly like that? (I only read through the code of #39870 today, but will look at it more in-depth on Monday).
There was a problem hiding this comment.
The thing with padding is that we only need it to be a variable at the root level, so we can easily apply it to all the places we need it: as padding on full-width root level blocks, or as negative margin on their full-width children. I'm not sure if block gap works exactly like that?
Block gap is a smidge different, for sure.
Support for gap in non root-level global styles was removed, and in sanitized() it's being excluded from the non-root level blocks and elements in the merged theme json. That means that, right now, there's no need to worry about whether the styles will be correctly output.
The side-effect of that removal was that block-level global styles cannot define gap values; this side-effect was, I believe, unintended. This is the "bug" that #39870 is trying to address.
This is a long-winded way of saying that I'd like to take block gap in the same direction you're taking padding 😄 so I think there might be similarities...
There was a problem hiding this comment.
There are many cases where we might want the site padding to match other parts of the site - for example the gaps between navigation items, template parts and galleries etc. This is an important consideration in theme design - if these spaces all defaulted to something that matched it would mean that designs would look good without needing to tweak all these things individually as we do today cc @kjellr @beafialho
There was a problem hiding this comment.
There are many cases where we might want the site padding to match other parts of the site - for example the gaps between navigation items, template parts and galleries etc.
That would be a theme level decision though, wouldn't it? I'd imagine the theme would set a global spacing variable and use it to set both root and block padding (or gap, or whatever else) in theme.json and in the theme stylesheet.
The root padding we're dealing with here is a top-level style so it's only attached to the body. This PR changes the output from a straightforward padding declaration to a custom property, mainly so we can use its value to offset full-width blocks.
There was a problem hiding this comment.
Yes, that's a theme level decision, but I think its the kind of thing that having a good default for would really help - for example if this setting defaulted to --wp--style--block-gap then all these spacing elements would match by default.
There was a problem hiding this comment.
Oh, I see!
I changed the default values here to use the block gap variable - does that work better?
| @@ -190,6 +191,10 @@ function getStylesDeclarations( blockStyles = {} ) { | |||
| [] | |||
| ); | |||
|
|
|||
| if ( isRoot ) { | |||
| return output; | |||
| } | |||
There was a problem hiding this comment.
This is a total hack 😅 to avoid the output of actual padding properties by the style engine logic below. Might be better to work out a way of getting the style engine to output the variables instead.
There was a problem hiding this comment.
I'm fine with the hack here, personally — it shouldn't be too hard to migrate the logic over to a separate file in the style engine package after the fact in a follow-up if you don't want to worry about it in this PR.
tellthemachines
requested review from
glendaviesnz,
ramonjd,
youknowriad,
andrewserong,
apeatling and
aaronrobertshaw
|
Nice one, thanks so much for taking this on, it's an important quality of life fix for block themes! Question: does it only work with root level full-wide blocks, though? I can't seem to get it to work in this featured image use case, where the featured image is part of post content: |
|
/cc @WordPress/block-themers |
| nodesWithStyles.forEach( ( { selector, styles } ) => { | ||
| const declarations = getStylesDeclarations( styles ); | ||
| const isRoot = ROOT_BLOCK_SELECTOR === selector; | ||
| const declarations = getStylesDeclarations( styles, isRoot ); |
There was a problem hiding this comment.
I'm doing something similar in the backend over at https://github.com/WordPress/gutenberg/pull/39870/files#diff-c220112e642d42482d91af8a96dca5ab587adfa705140148f86c064bca995631R184, but passing the whole selector.
|
Thanks for testing this @jasmussen! I'm not sure I fully get what's going on in that gif 😅 How does the Query Loop go inside the Post Content? Could you share some basic steps to set up that scenario?
This PR only addresses root level padding, and its effects on full width blocks that are direct children either of |
|
One thing that's not working quite as expected here is the top/bottom padding values added to |
tellthemachines
force-pushed
the
try/root-padding-fix
branch
from
1000c5d to
45c0f20
Compare
I haven't been following too closely recent changes to post content or query loop, but here's a GIF walking through activating the empty theme and then selecting the singular template and seeing the issue:
But I see the issue even if I drag the Query loop outside of the Post Content block:
Oh! My mistake. Ignore everything I said above 🙈 Thanks for working on this! |
| 'margin-right' => array( 'spacing', 'margin', 'right' ), | ||
| 'margin-bottom' => array( 'spacing', 'margin', 'bottom' ), | ||
| 'margin-left' => array( 'spacing', 'margin', 'left' ), | ||
| 'padding' => array( 'spacing', 'padding' ), |
There was a problem hiding this comment.
Should we give the same treatment to 'padding'?
If I add a padding value to theme.json,
"spacing": {
"blockGap": "1.5rem",
"padding": "88px"
},I see the following:
body {
padding: 88px;
--wp--style--block-gap: 1.5rem;
}Not sure what the intention is here, but If there were another CSS var for a single padding value, e.g., --wp--style--root--padding, we could use it as a fallback, e.g.,
padding-top: var(--wp--style--root--padding-top, var(--wp--style--root--padding))There was a problem hiding this comment.
Oh, good point, if the theme just sets "padding" my carefully crafted logic will fail 😂 😭
There was a problem hiding this comment.
We won't be able to use --wp--style--root--padding directly as a fallback though, because it might be a multi-value format e.g. 32px 42px. Not quite sure how best to handle that - is that something that we've solved for gap already?
There was a problem hiding this comment.
We won't be able to use --wp--style--root--padding directly as a fallback though, because it might be a multi-value format e.g. 32px 42px.
Oh, excellent point. 😆 Good catch.
I'm not sure splitting by ' ' would be robust enough (I tried in an old gap commit and was told the same).
As for gap, the gap value can be an array, so we check for that and split the values accordingly.
See: https://github.com/WordPress/gutenberg/blob/trunk/lib/block-supports/layout.php#L70
Otherwise we assume string.
There was a problem hiding this comment.
Hmmm, here we'll always have a string so that won't work.
If we could identify which of the variables are set, we could maybe output different styles accordingly, but I'm not sure how that would work on the editor side (the whole editor side of this PR is still quite iffy though, so maybe a solution will present itself in time)
There was a problem hiding this comment.
Thinking better of this, we always need to know the specific values for left and right padding because we need them for the negative margins on full width blocks. For that we need to separate the multi-value padding string into its component parts, so I think we'll have to do it the messy way.
There was a problem hiding this comment.
I updated the code below to separate out the values from a shorthand padding declaration. This currently only works on the server side; looking into how to implement it for the editor now.
| * @return {Array} An array of style declarations. | ||
| */ | ||
| function getStylesDeclarations( blockStyles = {} ) { | ||
| function getStylesDeclarations( blockStyles = {}, isRoot = false ) { |
There was a problem hiding this comment.
Ignorable observation: would it make sense to keep the selector arg consistent with how we're doing in the compute_style_properties() PHP method?
That is, passing the selector and doing the check inside this method?
ROOT_BLOCK_SELECTOR === selector
| } | ||
|
|
||
| // If a variable value is added to the root, the corresponding property should be removed. | ||
| foreach ( $root_variable_duplicates as $duplicate ) { |
There was a problem hiding this comment.
I've been playing with an alternative for block gap, in which I've created a second array const ROOT_PROPERTIES_METADATA that contains all the possible root properties.
In this case it would look like this:
const ROOT_PROPERTIES_METADATA = array(
// all the rest here...
// except "padding", "padding-top" etc, which are replaced by the following CSS vars
'--wp--style--root--padding' => array( 'spacing', 'padding' ),
'--wp--style--root--padding-top' => array( 'spacing', 'padding', 'top' ),
'--wp--style--root--padding-right' => array( 'spacing', 'padding', 'right' ),
'--wp--style--root--padding-bottom' => array( 'spacing', 'padding', 'bottom' ),
'--wp--style--root--padding-left' => array( 'spacing', 'padding', 'left' ),
);Then the only change to compute_style_properties() method would be:
f ( null === $properties ) {
$properties = static::ROOT_BLOCK_SELECTOR === $selector ? static::ROOT_PROPERTIES_METADATA : static::PROPERTIES_METADATA;
}Just noting for reference! It might be completely bonkers.
Seems to work for padding at least 😆 but haven't battle tested it yet. 🤔
There was a problem hiding this comment.
Ooooh that's a great idea! Much neater logic.
tellthemachines
force-pushed
the
try/root-padding-fix
branch
from
45c0f20 to
f18d4e1
Compare
| 'margin-right' => array( 'spacing', 'margin', 'right' ), | ||
| 'margin-bottom' => array( 'spacing', 'margin', 'bottom' ), | ||
| 'margin-left' => array( 'spacing', 'margin', 'left' ), | ||
| 'padding' => array( 'spacing', 'padding' ), |
There was a problem hiding this comment.
I updated the code below to separate out the values from a shorthand padding declaration. This currently only works on the server side; looking into how to implement it for the editor now.
| .is-root-container > .alignfull { | ||
| margin-right: calc(var(--wp--style--root--padding-right) * -1); | ||
| margin-left: calc(var(--wp--style--root--padding-left) * -1); | ||
| } |
There was a problem hiding this comment.
This is where it gets really tricky to handle shorthand padding values. Perhaps we can add this rule in dynamically somewhere else.
There was a problem hiding this comment.
If I'm not wrong it's not possible to have "align full" in the site editor (or any editor) unless there's a layout defined in the root which only happens in the post editor if the theme.json has "layout" key right? Makes me wonder if this should be moved to the edit-post or something like that to explain its purpose (maybe add a comment)
packages/blocks/src/api/constants.js
Outdated
| '--wp--style--padding-top': { | ||
| value: [ 'spacing', 'padding', 'top' ], | ||
| support: [ 'spacing', 'padding' ], | ||
| rootOnly: true, |
There was a problem hiding this comment.
Oh, thanks for the reminder @jasmussen ! I'd forgotten about the overlay special case. Do you know of anywhere else where we might need to apply root padding?
Nothing comes to mind, but I would note that I could see the overlay having a variety of behaviors some time the future, such as being an overlay that opens from the right and scrims the content, or a modal in the center. So it might not always need root padding. That is, it's probably fine to set it by default, and then let the user customize that padding on the modal overlay directly — and definitely a future concern, nothing for this PR, just context. |
tellthemachines
force-pushed
the
try/root-padding-fix
branch
from
cc14338 to
0b93772
Compare
There was a problem hiding this comment.
Here are a few things I noticed.
I'm using a full aligned image inside a group block with a layout defined.
- If I put my group block at the root level, everything works as expected on the frontend
- In the site editor root blocks I don't see any padding around random root blocks (not sure why).
- If I make my group block a template part, the full width images are not stretched anymore in the frontend.
In my singular template, I have a "post-content" block with layout but it's inside a group block to be able to apply some styling (background... and add some content after it...)
When I do that, full width aligned images in a post are not full width in the frontend, I can see the root padding around them.
| "right": "17px", | ||
| "bottom": "17px", | ||
| "left": "17px" | ||
| } |
There was a problem hiding this comment.
Is this like a demo padding that applies to all themes? won't this be a breaking change for existing themes?
| .is-root-container > .alignfull { | ||
| margin-right: calc(var(--wp--style--root--padding-right) * -1); | ||
| margin-left: calc(var(--wp--style--root--padding-left) * -1); | ||
| } |
There was a problem hiding this comment.
If I'm not wrong it's not possible to have "align full" in the site editor (or any editor) unless there's a layout defined in the root which only happens in the post editor if the theme.json has "layout" key right? Makes me wonder if this should be moved to the edit-post or something like that to explain its purpose (maybe add a comment)
| } else if ( !! properties && isString( styleValue ) && rootOnly ) { | ||
| const separateValues = styleValue.split( ' ' ); | ||
|
|
||
| const sortedBoxValues = { |
There was a problem hiding this comment.
What is this default value about?
There was a problem hiding this comment.
We should probably decide on a fallback value if neither theme nor user provides one; I just used 17px throughout this PR to make debugging easier. But here, I guess it could be 0 or even null, because we always fill in a value for all the properties below.
|
Thanks for testing @youknowriad !
That was a bug with variable names being kebab-cased, I've fixed it now.
This PR is only dealing with full-aligned blocks that are direct children of root blocks. When the group is wrapped in a template part, it's no longer a root block so the styles don't apply anymore.Non-root full width layouts are being addressed in #39871, as they require a different logic (I think; at least I haven't found a way to address both simultaneously 😅 )
That's the same problem as above with the nesting. I'm hoping that we might also be able to address that scenario in #39871. Will look into it better tomorrow! |
|
One note for people testing this - almost every theme already contains CSS to control this, so you will likely see some conflicts between that CSS and this PR unless you test with emptytheme. That's not to say we shouldn't test with other themes - we need to, to check that there aren't any regressions... |
|
I tested this with TT2 and on Archeo using this PR. It doesn't seem to be working correctly. Full width items aren't going to the browser edge, and don't have the correct padding applied. I see the problem in the single post template - the post content template has the "inherit default" property set, but then the CSS only targets direct children of |
Thanks for spending time on this. It's possible I borked something during the merge with trunk. It was a bit hairy due to all the compat changes for WordPress 6.0. I'll compare with a26beda and see if I can spot any omissions
That would be very useful, thank you! 😄 @tellthemachines is taking a break so it'd be great to keep this one rolling. |
I couldn't find anything glaring (I checked every change since the merge). The only spot I'm suspicious about is the root CSS here: https://github.com/WordPress/gutenberg/pull/39926/files#diff-1654d2da9097b71ac77806bbc505483b794371e8a2ff3ba152ec8610c2663ec6R438 The main challenge was merging after 49e4a4f... so maybe something needs to be updated to take into account that commit. |
I gave the latest version and can verify this on TT2. To add a little more context: As @scruffian mentions, this PR only targets direct children of
TT2's templates wrap the post content block inside of another group block. This is for good reason — to include a
The current alignment CSS in TT2 gets around this by being more general with its selector:
|
I added a commit to address this. It would be great if more people could test to find other issues that I missed :). Feel free to revert if it breaks stuff. |
| @@ -372,7 +372,7 @@ protected function get_block_classes( $style_nodes ) { | |||
| if ( $use_root_vars ) { | |||
| $block_rules .= '.wp-site-blocks { padding-top: var(--wp--style--root--padding-top); padding-bottom: var(--wp--style--root--padding-bottom); }'; | |||
| $block_rules .= '.wp-site-blocks > * { padding-right: var(--wp--style--root--padding-right); padding-left: var(--wp--style--root--padding-left); }'; | |||
| $block_rules .= '.wp-site-blocks > * > .alignfull { margin-right: calc(var(--wp--style--root--padding-right) * -1); margin-left: calc(var(--wp--style--root--padding-left) * -1); }'; | |||
| $block_rules .= '.wp-site-blocks > * .alignfull { margin-right: calc(var(--wp--style--root--padding-right) * -1); margin-left: calc(var(--wp--style--root--padding-left) * -1); padding-right: var(--wp--style--root--padding-right); padding-left: var(--wp--style--root--padding-left); width: unset; }'; | |||
There was a problem hiding this comment.
This basically targets all align full blocks at all levels. I'm not sure that's what we want, I thought this was only about the top level ones.
There was a problem hiding this comment.
The aim of the PR originally was just top level blocks and those inside post content. Blocks inside post content weren't working which is why I added this line. I know this will broaden the scope of the CSS, but this is the CSS that we have been using in all block themes, so it seems fairly safe to try.
lib/compat/wordpress-6.0/theme.json
Outdated
| @@ -240,6 +241,8 @@ | |||
| } | |||
| }, | |||
| "styles": { | |||
| "spacing": { "blockGap": "24px" } | |||
| "spacing": { | |||
| "blockGap": "24px" | |||
There was a problem hiding this comment.
I removed the default padding from here as it would impact all themes, which would be a problem.
|
I spent quite a bit of time on this today. In the end I took the CSS we currently use in block themes and then adapted it. This means we use the class names for several blocks that have different behaviours (columns, cover and group). I think it's necessary to give these specific blocks a different behaviour as we need to stop the content from touching the edges in these blocks. I'd be grateful to hear what others think. Can this also replace #39871? |
There was a problem hiding this comment.
Thanks a lot for picking this up and running with it @scruffian 🙇
It's mainly working as expected for me (aside from the doubling up on padding styles in body and .wp-site-blocks).
I'd be keen to hear from @youknowriad and others on whether the additional padding-specific changes in theme json class are okay, or whether they have the potential to create barriers to future refactoring.
It seems like it addresses a big headache for theme authors, so I think the tradeoffs could be worth it.
| if ( $use_root_vars ) { | ||
| $block_rules .= '.wp-site-blocks { padding-top: var(--wp--style--root--padding-top); padding-bottom: var(--wp--style--root--padding-bottom); }'; | ||
|
|
||
| $block_rules .= '.wp-site-blocks, |
There was a problem hiding this comment.
I'm wondering if we need to remove the padding applied directly to .wp-site-blocks
The padding set in theme.json and global styles are also set in the root selector (body) via static::to_ruleset above.
There was a problem hiding this comment.
Thanks for testing. I don't think they are applied to body, only the variables are defined on the body. In your screenshot one section is the vertical padding and the other is the horizontal padding :)
| ); | ||
|
|
||
| $declarations = array_merge( $declarations, $all_properties ); | ||
|
|
There was a problem hiding this comment.
I don't know if it's been brought up yet, but this is a lot of bespoke code for root padding values future iterations. I'm not sure where we'd put it in future iterations.
I'm not saying it's a blocker, just curious to learn whether we need a separate processing method for root block selectors to keep these methods uncluttered.
I'd speculate that root style CSS vars could be useful elsewhere, e.g., --wp--style--root--block-gap
There was a problem hiding this comment.
Yeah I agree a separate method for root selectors might make this tidier.
|
I squashed and rebased this and moved it to #40926 to keep the history here in tact. I think it's in a good place. |
scruffian
force-pushed
the
try/root-padding-fix
branch
from
dd60c36 to
0218267
Compare
|
I removed my commits from this so the original intent is clear. |
|
I tried combining this PR with #39871. The problem is that the way this works at the moment in trunk is that the outermost block that has "inherit default layout" has to cancel out the padding set on the root. This is challenging because the outermost block to "inherit default layout" is often not a child of ".wp-site-blocks". If we want to keep this behaviour, I think what we need is to find a way to determine if a block is wrapped in other "inherit default layout" blocks. If it isn't then it is the outermost block and needs to have negative margins to counter the root padding. |
|
Closing this PR as #42034 has an updated version of the same solution. |
* Layout blockGap: Try using classnames to support block-level gap in theme.json Try implementing partially in editor Try adding block classname to the container class to deal with specificity, remove fallback gap Add fallback gap styles rendered at root Move changes to theme.json class 'up' to 6-1 file Fix rendering block-level blockGap set in the block's attributes in the post editor Implement changes in site editor / global styles comparable to PHP changes Try moving some of the layout definitions to theme.json Move layout style generation to a separate function Implement theme.json definitions approach in the site editor, ensure styles load correctly in the post editor Remove class duplication, use classname stored in theme.json instead of hard-coded classname Support split row/column values at the block level, and include output of the legacy CSS variable for backwards compatibility Ensure CSS variable is only output if gap support is opted-in Tweak tests Whitespace fix Update test Remove duplication block class from server-rendered output, update snapshot Fix failing PHP test Attempt to fix PHP test again Manually fix snapshot Fix PHP linting issue Linting Reorganise rules in theme.json Remove dead code Render base styles and only output container classes and styles if unique values are generated Move blockGap styles in global styles to a separate getLayoutStyles function Move layout_definitions up so that it's always available to base styles Linting fixes Update test snapshot Add baseStyles output to global styles Remove test snapshot Update layout supports to return a CSS string instead of a component, add check that string is non-empty before outputting container classnames and style tags Update flex layout to only output styles if unique values are set Fix is-root-container styling in post editor Update flex/flow layouts to look up layout definitions to generate gap styling Move blockGap JS logic to a shared utility function, add tests Add test in case layoutDefinitions is undefined Add minimal tests that flex and flow layouts that don't contain non-default values return empty strings Fix rebase in 6-1.php file Consolidate JS layout classnames generation Further consolidate classname generation Implement outputting non-default layout gap for classic themes Update fallback gap logic so that block themes that opt-out of blockGap but opt-in to wp-block-styles still get flex layout gap styles Fix Columns fallback gap styles in classic themes Ensure base layout styles are available in the editor for classic themes Fix root gap value Fix linting issues Fix linting issue Add a phpunit test for outputting layout styles based on layout definitions in theme.json Add additional tests, ensure base styles are still output so that alignments continue to function Remove todo items Add layout selector regex, css declaration check Add additional logical margin properties to allow list Fix flex-wrap rule in JS version of flex layout Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Rename default_layout to global_layout_settings Rename blockGapStyles to spacingStyles Fix rebase issues Fix linting issues Fix linting again * Ensure blockGap controls are not exposed in global styles when experimental skip serializiation is used. This ensures that the Gallery block is not exposed in the global styles UI, as its blockGap values are proceeded individually at the block level. Support for the Gallery block in global styles will be looked into in future follow-ups. * Fix linting issue * Implement fallback behaviour in site editor where default flex gap is still rendered in themes without blockGap but with wp-block-styles * Remove connection to wp-block-styles so that fallback flex layout styles are always output * Update resolver class to add an empty blockGap placeholder for a block, if it provides a default blockGap value * Layout blockGap: Try using classnames to support block-level gap in theme.json Try implementing partially in editor Try adding block classname to the container class to deal with specificity, remove fallback gap Add fallback gap styles rendered at root Move changes to theme.json class 'up' to 6-1 file Fix rendering block-level blockGap set in the block's attributes in the post editor Implement changes in site editor / global styles comparable to PHP changes Try moving some of the layout definitions to theme.json Move layout style generation to a separate function Implement theme.json definitions approach in the site editor, ensure styles load correctly in the post editor Remove class duplication, use classname stored in theme.json instead of hard-coded classname Support split row/column values at the block level, and include output of the legacy CSS variable for backwards compatibility Ensure CSS variable is only output if gap support is opted-in Tweak tests Whitespace fix Update test Remove duplication block class from server-rendered output, update snapshot Fix failing PHP test Attempt to fix PHP test again Manually fix snapshot Fix PHP linting issue Linting Reorganise rules in theme.json Remove dead code Render base styles and only output container classes and styles if unique values are generated Move blockGap styles in global styles to a separate getLayoutStyles function Move layout_definitions up so that it's always available to base styles Linting fixes Update test snapshot Add baseStyles output to global styles Remove test snapshot Update layout supports to return a CSS string instead of a component, add check that string is non-empty before outputting container classnames and style tags Update flex layout to only output styles if unique values are set Fix is-root-container styling in post editor Update flex/flow layouts to look up layout definitions to generate gap styling Move blockGap JS logic to a shared utility function, add tests Add test in case layoutDefinitions is undefined Add minimal tests that flex and flow layouts that don't contain non-default values return empty strings Fix rebase in 6-1.php file Consolidate JS layout classnames generation Further consolidate classname generation Implement outputting non-default layout gap for classic themes Update fallback gap logic so that block themes that opt-out of blockGap but opt-in to wp-block-styles still get flex layout gap styles Fix Columns fallback gap styles in classic themes Ensure base layout styles are available in the editor for classic themes Fix root gap value Fix linting issues Fix linting issue Add a phpunit test for outputting layout styles based on layout definitions in theme.json Add additional tests, ensure base styles are still output so that alignments continue to function Remove todo items Add layout selector regex, css declaration check Add additional logical margin properties to allow list Fix flex-wrap rule in JS version of flex layout Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Rename default_layout to global_layout_settings Rename blockGapStyles to spacingStyles Fix rebase issues Fix linting issues Fix linting again * Reuse most of the logic from #39926 * Don't accept string values * Apply root padding only on blocks with content width * Apply linting changes * Fix rebase error * Actually fix rebase * Add global padding toggle to layout-less blocks * Support custom block padding. * Output alignfull styles only when needed. * Toggle should only appear when needed. * Add context to comments about string value support * Interpret preset padding value for negative margin * Don't show toggle on blocks without content size. * Change setting name. * Improve preset processing logic * Reset padding when no content size applies. * Update style engine class to match trunk * Fix string check * Fix preset values in the editor. * Fix string check * Remove useGlobalPadding attribute * Replace padding with spacing in function parameter * Default root padding setting to false. * Dodgy fix for Cover block * Add test for get_styles_for_block function * Check for padding setting before adding classname. * Don't output vars if setting is off * Test default output without setting enabled. * Update lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php No humorous string values allowed Co-authored-by: Ramon <ramonjd@users.noreply.github.com> * Remove empty line. Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
What?
theme.jsonso themes can opt in to the new behaviour of root paddingWhy?
Partly fixes #35607 - only addresses the root block part of the equation. #39871 deals with full width in nested blocks.
Fixes #35884 for themes that opt into this change.
Themes shouldn't have to add their own rules to make full width alignments work properly.
How?
First, we're setting a default padding value in theme.json. It can be overridden by any user-set padding in the global styles interface.
Then, on the server side, we set top and bottom padding to the root container, but left and right padding to the direct children of root. This is so that those children still stretch full width as expected, but their children get the effects of the padding.
On the site editor side, the direct children of root container get the left/right padding applied, so they still appear as full-width. (If we wanted to only apply full-width to some of the root children, maybe we could add padding to the root and enable the align control on them so they could be set manually? And add negative margins to
alignfullblocks)On the post editor side, the root container also gets its left/right padding, and any
alignfullchildren are fixed with negative margins.Testing Instructions
"useRootVariables": trueto thesettingsof your theme'stheme.jsonto enable these changes."useRootVariables": truefromtheme.jsonreverts to the previous behaviour.Screenshots or screencast