Gallery: register styles with style engine

[ramonjd]

Depends on:

• Style engine: permit wp custom CSS properties #43071
• Update 6.1 CSS filter to match core #44962

What?

Update gallery to use style engine enqueuing, and also WP_HTML_Tag_Processor to add the unique class.

Why?

• To batch styles in one style tag.
• Also, gutenberg_enqueue_block_support_styles() will soon be deprecated.

Testing Instructions

Since https://core.trac.wordpress.org/ticket/56353 WordPress supports CSS custom vars. It will be shipped with 6.1.

For sites running < 6.1 Gutenberg allows specific CSS properties only. See: #43071

1. Create a new gallery, add your favourite holiday pics.
2. Apply a block gap value to your gallery and publish!
3. Check that your gallery appears as you'd expect it to on the frontend, and that it has the following styles (with your block gap value):

<style id='block-supports-inline-css'>
/* ...some styles */
.wp-block-gallery.wp-block-gallery-1 {
	--wp--style--unstable-gallery-gap: 107px;
	gap: 107px;
}
/* ...some styles */
</style>

Test in classic and block themes.

[ramonjd]

I think we might have to come up with a way to ensure these styles are printed after the layout styles:

See:

• Lower the priority of the gallery gap css so it loads after the block layout css #41423

cc @glendaviesnz

[andrewserong]

Following the testing in #41423, it looks like the fallback styling rules aren't being output on a classic theme for me (TwentyTwentyOne), so if I add the following to my theme's css, it doesn't affect the gallery gap:

html {
	--wp--style--gallery-gap-default: 37px;
}

From a bit of introspection, it appears that the fallback rules in $fallback_gap are being stripped in WP 6.0, as it's a pretty complex value, and the gutenberg_safecss_filter_attr_allow_css_6_1 function here hasn't been updated to reflect the final version in 6.1 on these lines: https://github.com/WordPress/wordpress-develop/blob/2b4d385298504d412e9f0dea2e7019d53463c705/src/wp-includes/kses.php#L2508-L2512

If I copy over that preg_replace line to the Gutenberg function, it appears that the style output works correctly, so we might need to land another backport of a backport before we can go with this PR?

[andrewserong]

I'll put up a PR to backport that line.

[ramonjd]

Interesting...

Seeing duplicate comments in the block supports inline CSS

<style id='core-block-supports-inline-css'>
/**
 * Core styles: block-supports
 */

/**
 * Core styles: block-supports
 */

Might mean that the hook callback is running twice

👀

[andrewserong]

Thanks for keeping this one current @ramonjd! It's mostly working, but I ran into an issue with fallback gap on classic themes. I think it's uncovered a missing backport of allowed css functions in Gutenberg, but I've left a comment where we'll likely need to follow-up there.

[andrewserong]

Should we remove this deprecation notice since it didn't make it into 6.1, and instead add it back in once we're ready to start adding in 6.2 versions of these functions?

[ramonjd]

Oh yeah 🤦 This PR is old.

Will do, thank you!

[ramonjd]

Looks like there's a wordpress-6.2/script-loader.php file now. I could move it there and keep the notice, and bump it to 6.2

[andrewserong]

Following the testing in #41423, it looks like the fallback styling rules aren't being output on a classic theme for me (TwentyTwentyOne), so if I add the following to my theme's css, it doesn't affect the gallery gap:

html {
	--wp--style--gallery-gap-default: 37px;
}

From a bit of introspection, it appears that the fallback rules in $fallback_gap are being stripped in WP 6.0, as it's a pretty complex value, and the gutenberg_safecss_filter_attr_allow_css_6_1 function here hasn't been updated to reflect the final version in 6.1 on these lines: https://github.com/WordPress/wordpress-develop/blob/2b4d385298504d412e9f0dea2e7019d53463c705/src/wp-includes/kses.php#L2508-L2512

If I copy over that preg_replace line to the Gutenberg function, it appears that the style output works correctly, so we might need to land another backport of a backport before we can go with this PR?

[ramonjd]

If I copy over that preg_replace line to the Gutenberg function, it appears that the style output works correctly, so we might need to land another backport of a backport before we can go with this PR?

Great sleuthing, thanks @andrewserong

I'll do the backport in another PR since it's bit separate and come back and test the scenario.

I appreciate the test! 🙇

[andrewserong]

I'll do the backport in another PR since it's bit separate and come back and test the scenario.

Oh, thanks! I've opened up a backport PR already in #44962 — I mightn't get a chance to test it properly today (I just copy + pasted and hoped for the best for the moment), so might leave it until next week to try to get it merged, if it isn't too urgent

[ramonjd]

Oh, thanks! I've opened up a backport PR already in #44962 — I mightn't get a chance to test it properly today (I just copy + pasted and hoped for the best for the moment), so might leave it until next week to try to get it merged, if it isn't too urgent

Merged!

Thanks a lot for the quick PR

[glendaviesnz]

I think we might have to come up with a way to ensure these styles are printed after the layout styles

The styles seemed to work ok for me on Classic themes. I think #41015 removed the need to worry about the priority.

This tested pretty well for me. The only thing I noticed is that setting the gap via either the deprecated --gallery-block--gutter-size or --wp--style--gallery-gap-default in a classic theme applied the gap in the frontend but not in the editor, but it does that for me on trunk as well, so don't think it is related to this PR. Do you also see this on trunk?

[ramonjd]

The only thing I noticed is that setting the gap via either the deprecated --gallery-block--gutter-size or --wp--style--gallery-gap-default in a classic theme applied the gap in the frontend but not in the editor, but it does that for me on trunk as well, so don't think it is related to this PR. Do you also see this on trunk?

Thanks for testing @glendaviesnz

I activated 2021 theme, then added the following to ./style.css

:root {
--wp--style--gallery-gap-default: 22px;
--gallery-block--gutter-size:11px;
}

This indeed worked for the frontend, but not the editor.

I had to add the CSS to ./assets/css/style-editor.css to get it to work everywhere:

Is that expected. Which classic theme did you test with?

[glendaviesnz]

This indeed worked for the frontend, but not the editor.

I had to add the CSS to ./assets/css/style-editor.css to get it to work everywhere
Is that expected. Which classic theme did you test with?

Given that the likes of TwentyTwentyOne have lots of CSS vars in ./assets/css/style-editor.css I am picking we can assume it is normal to have to do that to get any extra vars to work in the editor, so let's leave it as that for now.

[glendaviesnz]

This tested well for me with block-based and classic themes.

[Mamaduka]

@ramonjd, is there a separate core ticket for deprecating wp_enqueue_block_support_styles?

[andrewserong]

@Mamaduka, I don't think there is a separate core ticket as far as I'm aware. @ramonjd isn't around at the moment, but I can open a trac ticket, I think I've got enough context to describe the intention here.

[andrewserong]

@Mamaduka @glendaviesnz I've opened up a trac ticket to flag the deprecation of the function: https://core.trac.wordpress.org/ticket/57647

The short version is: as of 6.2 all usage of wp_enqueue_block_support_styles will have been replaced with calls to the style engine, where styles are grouped together and output in a single style tag. Prior to using the style engine, the calls to wp_enqueue_block_support_styles resulted in excess style tags being output to the site frontend, along with redundant CSS rules.

As far as I can tell, we can't deprecate the function until after the JS packages update has happened, as that change will include the switch in the gallery block's index.php file reflected in this PR. That appears to be the last usage of wp_enqueue_block_support_styles, so once this change is backported, we should then be right to add in the _deprecated_function() call to deprecate the function. I'll open up a wordpress-develop PR so that the deprecation is ready for review once the JS packages are updated.

[andrewserong]

Update: I've opened up a PR to deprecate wp_enqueue_block_support_styles over in WordPress/wordpress-develop#4015 — I believe that one should only land after the JS packages update, if I'm following correctly 🙂