Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid enqueuing global styles twice when running on WordPress 5.8 #32372

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Jun 1, 2021

The Gutenberg plugin embeds a stylesheet that contains the global styles of the site. WordPress 5.8 does the same. So, when the plugin runs on WordPress 5.8 we need to tell WordPress core not to enqueue its styles to prevent conflicts and avoid enqueueing them twice.

Types of changes

If there's a global-styles style handle, the plugin will overwrite the contents of the first style element (styles that come from WordPress core). If there's not, it'll enqueue a new stylesheet with that style handle.

How to test

Test the following scenarios and verify that when visiting the front end, there's a global-styles-inline-css stylesheet.

  • WordPress <5.8 with Gutenberg
  • WordPress 5.8 with and without Gutenberg

Alternatives considered

An alternative I've tried (see aa6e0fa) was to make the plugin use a different style handle (global-styles-gutenberg) and deregister core's if there was one. However, maintaining the style handle consistent allows third-party to hook into it as well.

as the plugin will take care of enqueuing what's necessary.
@oandregal oandregal requested a review from spacedmonkey as a code owner June 1, 2021 13:00
@oandregal oandregal self-assigned this Jun 1, 2021
@oandregal oandregal requested review from jorgefilipecosta and a team June 1, 2021 13:01
@oandregal oandregal added [Type] Code Quality Issues or PRs that relate to code quality Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Jun 1, 2021
@oandregal oandregal changed the title Avoid enqueuing global styles twice when using WordPress 5.8 Avoid enqueuing global styles twice when running on WordPress 5.8 Jun 1, 2021
lib/global-styles.php Outdated Show resolved Hide resolved
Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good and works as advertised 👍

@aristath
Copy link
Member

aristath commented Jun 1, 2021

Just an idea... Instead of removing global-styles and adding global-styles-gutenberg, we could do something like this to switch the data:

if ( isset( wp_styles()->registered['global-styles'] ) ) {
	wp_styles()->registered['global-styles']->extra['after'][0] = $stylesheet;
} else {
	wp_register_style( 'global-styles', false, array(), true, true );
	wp_add_inline_style( 'global-styles', $stylesheet );
}

edit: the priority of the hook may need to be changed if we go with something like the above, just to be sure it runs after the global-styles stylesheet is added

@oandregal
Copy link
Member Author

@aristath that sounds exactly like what my first instinct was but didn't find how to do it. Going to give it a try, thanks for sharing!

wp_add_inline_style( 'global-styles', $stylesheet );
wp_enqueue_style( 'global-styles' );
if ( isset( wp_styles()->registered['global-styles'] ) ) {
wp_styles()->registered['global-styles']->extra['after'][0] = $stylesheet;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only overwrites the first element, which represents styles that come from WordPress core, so it preservess any other styles that may have been registered after.

@oandregal
Copy link
Member Author

@aristath I've implemented your suggestion, it works nicely and we seem to have used a similar approach in other places as well. Maintaining the global-styles handle consistent no matter the runtime/plugin combination is a good call as well.

Going to merge after test pass, unless you have other feedback.

@aristath
Copy link
Member

aristath commented Jun 1, 2021

Tested again and everything works as expected ❤️
No objections, this should be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants