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

Revert "Update function names for the public global styles API functions" #36957

Closed
wants to merge 1 commit into from

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Nov 29, 2021

Reverts #36907

What this PR fixes

When the plugin is active in WordPress 5.9, it doesn't override global styles WordPress core public API.

Context

In #36907 I renamed the public API of GS from gutengerg_* to wp_*), to make backport easier and make the functions available in all the WordPress versions the plugin supports. I hoped that we could introduce some filters at #36909 to modify their behavior from the plugin. However, we decided against that, and, as a side-effect, we don't have a way to overwrite the WordPress core functions when the plugin runs on WordPress 5.9.

Follow-ups post 5.9

We'd need to clarify how to best address the following (can be done post-5.9):

  • What happens when/if we need to modify the behavior of the public GS API? In principle, we shouldn't update code that lives in lib/compat/5.9 after WordPress 5.9 has been released.

  • What happens if we don't need to modify the behavior of those functions and the plugin minimum version is 5.9 (so the lib/compat/5.9 is removed? At that point, we can't remove the functions from the plugin as we may still need them to overwrite the core behavior.

Unless I'm missing something, it looks like the code for the public API of GS should not live in lib/compat.

@oandregal oandregal self-assigned this Nov 29, 2021
@oandregal oandregal added Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Nov 29, 2021
@oandregal oandregal marked this pull request as ready for review November 29, 2021 13:12
Copy link
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

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

I tested some of this whilst working on #36960 and it allowed me to make changes to the lib/class-wp-theme-json-gutenberg.php file where previously it would only load the WP Core version.

@oandregal
Copy link
Member Author

oandregal commented Nov 29, 2021

I'm going to close this.

I ran this by some folks for thoughts (Riad, Jorge) and this is the consensus:

  • Any global styles change aimed for 5.9 needs two PRs: one for the plugin and another for WordPress trunk.
  • The changes to the plugin should be tested with WordPress 5.8, as they won't work with WordPress trunk (the plugin won't register the functions if WordPress core already did, see).
  • If, and only if, there's a change that is not aimed for 5.9 we'll look into overriding the global styles code that lives in lib/compat/5.9.

The rationale for this is that we want to remove as much code from the plugin as possible and this is another step in that direction for global styles.

cc some folks I know contributed to the GS code for awareness @getdave @noisysocks @aaronrobertshaw @andrewserong @scruffian @fabiankaegy @Mamaduka @carolinan @ajlende @aristath @walbo

@oandregal oandregal closed this Nov 29, 2021
@oandregal oandregal deleted the fix/gutenberg-on-5-9 branch November 29, 2021 15:45
@oandregal oandregal removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 29, 2021
@oandregal
Copy link
Member Author

Draft PR to move the global styles related code under lib/compat/wordpress-5.9 folder #36978

@oandregal
Copy link
Member Author

#36978 is ready for review.

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants