Use the Style Engine in layout block-supports

[aristath]

What?

#40875 refactored the layout.php block-supports to be more efficient. In #42043 we introduced a WP_Style_Engine_CSS_Declarations object, so this PR makes use of the declarations object in the refactored layout file.

Why?

Use the Style Engine where appropriate.

How?

Use an array of selectors along with their declarations, then compile that to get the final string.

Testing Instructions

Load a page in the twentytwentytwo theme (or any other block theme) and check that the layout styles are there. Should be pretty easy to check, if the styles are missing the layout is broken 😆

[andrewserong]

Nice work @aristath, thanks for putting this together so quickly now that the big Layout refactor PR's been merged!

This is working nicely for me, and it feels much clearer building up a declarations array rather than the existing string concatenation, and makes for a great use case of the declarations class you just landed recently 🎉

Each of the different layout options is rendering correctly for me in manual testing:

✅ Group block content width is output correctly
✅ Group block wide width is output correctly
✅ Group block center alignment is output correctly (looks like we're passing a redundant semi-colon, though)
✅ Group block blockGap is output correctly
✅ Buttons blockGap is output correctly
✅ Buttons horizontal content justification is output correctly
✅ Buttons horizontal vertical alignment is output correctly
✅ Buttons vertical content justification is output correctly

For visibility there's another PR (#42085) from @tellthemachines that's looking to make an addition to this function, with a rule for negative margins for .alignfull. The rule uses calc(), which I think should pass the sanitization in get_declarations_string, but just thought I'd double-check if you think that'll cause any issues?

Otherwise, this looks good to go to me, I'll just give the others a chance to chime in before giving an approving review tomorrow, if no-one beats me to it!

[ramonjd]

Also big thanks for this PR! 🙇 I think it's a good use of the declarations class.

I’m just wondering if it’d be neater to aim to get the store finished. That way we could move straight to storing/enqueuing via an abstraction method rather than use a style engine class directly in the Gutenberg code?

Then again, we have some time before 6.1 and things will change before we merge this file to core, so it’s probably not a huge deal.

[ramonjd]

I’m just wondering if it’d be neater to aim to get the store finished. That way we could move straight to storing/enqueuing via an abstraction method rather than use a style engine class directly in the Gutenberg code?

Here's a rough implementation using elements and @aristath's work over in #42222

e14fbc2

It works 🎉

I'll keep looking at the enqueuing stuff regardless of whether we merge this PR.

🍺

[andrewserong]

For visibility there's another PR that's looking to make an addition to this function, with a rule for negative margins for .alignfull. The rule uses calc(), which I think should pass the sanitization in get_declarations_string, but just thought I'd double-check...

It looks like #42085 is probably going to be merged shortly, so once that's in, this PR will need a rebase, and we can re-confirm that it's going to work with the additional calc() rule. (Seems like it should be fine 🤞)

I’m just wondering if it’d be neater to aim to get the store finished. That way we could move straight to storing/enqueuing via an abstraction method rather than use a style engine class directly in the Gutenberg code?

I don't mind too much either way if we go with this approach first, or wait until we know how we'd like to do the enqueueing, but it's been good focusing on the store work in #42222 — if we do land this one first, it'd be nice to focus on the enqueueing as a quick follow-up. Thanks for putting together #42452 @ramonjd — I ran out of time to test that one today, but I'll give it whirl on Monday 🙇

[aristath]

I pushed some commits here to use the Style Engine Store instead of the Declarations object (tested and it works) 👍
I'd like to wait though until we get the Renderer in place (which is why I drafted this PR for now).
In #42463 I added a Renderer object along with an implementation to combine selectors that have the same declarations. If that implementation gets merged, then the impact on the generated styles for layouts would be huge!

[ramonjd]

I pushed some commits here to use the Style Engine Store instead of the Declarations object (tested and it works) 👍

Nice to see a working example of the store!

Once the renderer and other pieces are in place, my vote would be to abstract things a bit further, even only if it's so that consumers don't need to know about any store keys.

This'll be based on need I suppose, but I imagine that we'd also want to do something similar – adding non-base block-supports styles that don't need parsing to a store – over in global styles.

I'd like to wait though until we get the Renderer in place (which is why I drafted this PR for now).

👍 👀

[aristath]

This part will be replaced by the Processor/Renderer implementation when that gets merged

[ramonjd]

This might not be ideal since we'd be enqueuing block supports styles from many places. Wouldn't this pull out all of them, not just layout-related?

This is why I think having an abstracted method, that uses the classes in the background, and that centralizes the work, could be a better option.

Example: https://github.com/WordPress/gutenberg/pull/42452/files#diff-69616e5e4ff1a06362db0b7165c330b00614d4ea0d3a28c80003c0860ab8b93fR683

[aristath]

Hmmm you're right. We'll need to make sure that all these styles only get printed once 👍

[aristath]

What if we make the store names a bit more specific for the time being until we have a more abstract & generalized method to handle these? So instead of having a block-supports store, we could name it block-supports/layout. This would make it more specific and each store can be handled differently until we have a more generic strategy 🤔

[ramonjd]

True, it's a bit tricky. I think we'll have to experiment a bit before we get it right. 🤔

[aristath]

After merging the Processor class, I refactored this PR to use it.

Things that I needed to change:

• Added a gutenberg_set_layout_style function. The gutenberg_get_layout_style function now uses that, but it's not actually used anywhere in the code and could/should be deleted. I kept it for backwards-compatibility, but I'm not sure it's needed.
• Added a gutenberg_enqueue_style_engine_store function. This will print the styles from a store (accepts the store-name as an arg).
• When the store prints CSS, the rules objects are purged from it to prevent duplicate styles in case we somehow manage to enqueue the same store multiple times.

The CSS printed from the layouts in `trunk`

<style>.wp-block-navigation.wp-container-3 {justify-content: flex-end;}</style> <style>.wp-block-group.wp-container-4 {justify-content: space-between;}</style> <style>.wp-block-group.wp-container-5 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-5 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-5 .alignfull { max-width: none; }</style> <style>.wp-block-group.wp-container-6 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-6 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-6 .alignfull { max-width: none; }</style> <style>.wp-block-columns.wp-container-9 { flex-wrap: nowrap; }</style> <style>.wp-block-group.wp-container-10 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-10 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-10 .alignfull { max-width: none; }</style> <style>.wp-block-columns.wp-container-13 { flex-wrap: nowrap; }</style> <style>.wp-block-group.wp-container-14 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-14 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-14 .alignfull { max-width: none; }</style> <style>.wp-block-columns.wp-container-17 { flex-wrap: nowrap; }</style> <style>.wp-block-group.wp-container-18 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-18 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-18 .alignfull { max-width: none; }</style> <style>.wp-block-columns.wp-container-21 { flex-wrap: nowrap; }</style> <style>.wp-block-group.wp-container-22 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-22 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-22 .alignfull { max-width: none; }</style> <style>.wp-block-columns.wp-container-25 { flex-wrap: nowrap; }</style> <style>.wp-block-group.wp-container-26 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-26 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-26 .alignfull { max-width: none; }</style> <style>.wp-block-query-pagination.wp-container-28 {justify-content: space-between;}</style> <style>.wp-block-query.wp-container-29 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-query.wp-container-29 > .alignwide { max-width: 1000px;}.wp-block-query.wp-container-29 .alignfull { max-width: none; }</style> <style>.wp-block-group.wp-container-30 {justify-content: space-between;}</style> <style>.wp-block-group.wp-container-31 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-31 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-31 .alignfull { max-width: none; }</style> <style>.wp-block-group.wp-container-32 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px;margin-left: auto !important;margin-right: auto !important;}.wp-block-group.wp-container-32 > .alignwide { max-width: 1000px;}.wp-block-group.wp-container-32 .alignfull { max-width: none; }</style>

The CSS printed from layouts in this PR

<style id="wp-style-engine-block-supports-layout">.wp-block-navigation.wp-container-3 {justify-content: flex-end;}.wp-block-group.wp-container-4,.wp-block-query-pagination.wp-container-28,.wp-block-group.wp-container-30 {justify-content: space-between;}.wp-block-group.wp-container-5 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-group.wp-container-6 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-group.wp-container-10 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-group.wp-container-14 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-group.wp-container-18 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-group.wp-container-22 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-group.wp-container-26 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-query.wp-container-29 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-group.wp-container-31 > :where(:not(.alignleft):not(.alignright):not(.alignfull)),.wp-block-group.wp-container-32 > :where(:not(.alignleft):not(.alignright):not(.alignfull)) {max-width: 650px; margin-left: auto !important; margin-right: auto !important;}.wp-block-group.wp-container-5 > .alignwide,.wp-block-group.wp-container-6 > .alignwide,.wp-block-group.wp-container-10 > .alignwide,.wp-block-group.wp-container-14 > .alignwide,.wp-block-group.wp-container-18 > .alignwide,.wp-block-group.wp-container-22 > .alignwide,.wp-block-group.wp-container-26 > .alignwide,.wp-block-query.wp-container-29 > .alignwide,.wp-block-group.wp-container-31 > .alignwide,.wp-block-group.wp-container-32 > .alignwide {max-width: 1000px;}.wp-block-group.wp-container-5 .alignfull,.wp-block-group.wp-container-6 .alignfull,.wp-block-group.wp-container-10 .alignfull,.wp-block-group.wp-container-14 .alignfull,.wp-block-group.wp-container-18 .alignfull,.wp-block-group.wp-container-22 .alignfull,.wp-block-group.wp-container-26 .alignfull,.wp-block-query.wp-container-29 .alignfull,.wp-block-group.wp-container-31 .alignfull,.wp-block-group.wp-container-32 .alignfull {max-width: none;}.wp-block-columns.wp-container-9,.wp-block-columns.wp-container-13,.wp-block-columns.wp-container-17,.wp-block-columns.wp-container-21,.wp-block-columns.wp-container-25 {flex-wrap: nowrap;}</style>

The overall reduction in size is ~38%, and everything still works the way it's supposed to 🎉

[andrewserong]

Nice one, very promising seeing the reduction of the size of the CSS output! 🎉 Just added a couple of comments and thoughts from looking at some overlapping work surrounding enqueueing in #42452

[andrewserong]

Oh, this is an interesting way to solve it. Will this effectively involve registering dozens of stores if a post contains dozens of blocks that opt-in to layout support?

It looks like @ramonjd is attempting to solve a similar problem as this PR in #42452. In that PR there's a wp_style_engine_enqueue_block_supports_styles function (here) which would call a method of WP_Style_Engine to add the declarations to the block-supports store. If I'm following that PR correctly, when the main WP_Style_Engine class is constructed, it registers the hooks for output, so the behaviour is encapsulated a bit further. I think the result then would be that we'd only need the one block-supports store, and it gets output once? Though there's a bit of discussion here (https://github.com/WordPress/gutenberg/pull/42452/files#r922931702) that for that process to work, we might need to flag at a store level whether or not it's to be enqueued?

So many little bits to figure out 😀

[aristath]

Will this effectively involve registering dozens of stores if a post contains dozens of blocks that opt-in to layout support?

Nope! The gutenberg_get_layout_style is no longer used anywhere and can probably be removed. I tweaked it here to use a unique store per selector just to cover all bases in case someone uses the function in a plugin etc. So it's basically just for backwards-compatibility purposes 😉

[andrewserong]

Ah, thanks for clarifying, I was probably reading too quickly! Good idea thinking of backwards compatibility — I wonder if in practice anyone actually uses it directly? I think I'd always thought of it as an internal / de-facto private method, but yeah, there very well could be someone attempting to use it in a plugin or theme 🤔

[ramonjd]

If I'm following that PR correctly, when the main WP_Style_Engine class is constructed, it registers the hooks for output, so the behaviour is encapsulated a bit further. I think the result then would be that we'd only need the one block-supports store, and it gets output once?

That's how I've built it 👍

Is there a reason why we'd want multiple stores for block supports?

I'm open to accusations of egocentric bias, but I think #42452 might do want we want without the need for granular stores. I've tested it using the CSS storing steps in this PR.

we might need to flag at a store level whether or not it's to be enqueued?

I've solved this by breaking apart the compile (into CSS strings) and parser (parsing raw block style values) functionalities, which should set us up for further abstraction.

I wonder if in practice anyone actually uses it directly? I think I'd always thought of it as an internal / de-facto private method, but yeah, there very well could be someone attempting to use it in a plugin or theme

I reckon we should encourage using the internal method in practice (in code and eventually in documentation), and I think it'd be neater for our own purposes anyway because

1. The use of the classname is abstracted
2. The store key exists in one place (fewer places to update)
3. Abstracting logic should mean that making changes (which will be inevitable during these early phases) will give us fewer headaches.

If theme authors really want to hook into Gutenberg's stores, they'll be able to do it via any filters we build in later as demonstrated in #42493

[ramonjd]

Is there a reason why we'd want multiple stores for block supports?

Ooooh! I see that we'd have to return the layout styles in gutenberg_get_layout_style for backwards compatibility. 🤦 It is a bit fiddly.

It'd still be nice not to have to expose the classes too early.

I'm sure we'll come up with something.

[andrewserong]

Do we need to create a new function, or can the logic that's now in gutenberg_get_layout_style move to render_layout_support_flag? It can be hard to remove these global-namespace functions once they're introduced, so it'd be good to see if there are ways to avoid adding additional functions where we can.

Separately to hooking in the style engine, one of the tasks of #39336 is to try moving content justification to a preset-like approach, so that part of the logic might eventually be removed from the body of this function — so (in theory at least) we might be able to eventually reduce the complexity of this function a little bit. That can totally happen well after this PR lands, though, so not a blocker for the work here.

[ramonjd]

I think I'm starting to come around to leaving the work of enqueuing to the consumer like this (I'm slow 😄) especially in the case of Gutenberg, where there are existing functions that we can repurpose.

Maybe later we could think about writing public utility methods in the style engine to take some of the work of enqueuing away from other consumers.

The gutenberg_enqueue_block_support_styles function above is another great example of where we could reuse/recycle, especially given the gymnastics we seem to have to go through to ensure backwards compatibility for such functions.

[aristath]

Closing because #42452 got merged instead.