gutenberg icon indicating copy to clipboard operation
gutenberg copied to clipboard

Global Styles: Alternative method for enqueueing custom CSS

Open Mamaduka opened this issue 2 years ago • 1 comments

What?

My alternative to the enqueueing method is proposed at https://github.com/WordPress/gutenberg/pull/47396.

Instead of using two separate callbacks for loading custom CSS from Customizer and Site Editor, the new method removes Customzer callback, combines custom CSS values, and enqueues styles after global-styles.

I've also made the following changes to the get_global_styles_custom_css method:

  • Added prefix to avoid conflicts when the function is backported.
  • Removed checks for the $types variable - the function doesn't accept arguments.
  • Update the cache key name and added it to the clean function.
  • Avoid calling the ::get_merged_data if a theme does not have a theme.json file.

Testing Instructions

Please take a look at the instructions in the original PR.

Mamaduka avatar Jan 30 '23 13:01 Mamaduka

Flaky tests detected in d552d794f5cb68be062bd6c08720374efca812f9. Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4053536069 📝 Reported issues:

  • #36495 in specs/editor/various/links.test.js
  • #35176 in specs/editor/various/autosave.test.js

github-actions[bot] avatar Jan 30 '23 13:01 github-actions[bot]

This tested well for me. The only thing I noticed is that with using wp_add_inline_style within wp_enqueue_scripts instead of echoing the style in wp_head it places the styles higher up in the header than the default customizer output which is towards the very end of the header. I think there would only be a very minimal risk of this causing any specificity issues with any existing hybrid themes with customizer custom CSS set so don't think it is too critical, what do you think?

A comment was also made here about trying to keep the custom CSS loading in the same location as the customizer CSS - I don't have a strong opinion one way or the other on how critical this is - use of the wp_head and manually echoing out the <style> element seems like the only way to do this, and this seems to be a little hacky.

glendaviesnz avatar Jan 30 '23 22:01 glendaviesnz

Using wp_head with high priority doesn't guarantee that Global Styles's custom CSS will be loaded last or even after global-styles.

Let's keep using wp_add_inline_style for now. Then, we can change the callback hook and printing method based on feedback. What do you think?

cc @aristath

Mamaduka avatar Jan 31 '23 04:01 Mamaduka

Let's keep using wp_add_inline_style for now. Then, we can change the callback hook and printing method based on feedback. What do you think?

Sounds good to me 👍

aristath avatar Jan 31 '23 06:01 aristath