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

Migrate theme.json based on origin #62305

Merged
merged 7 commits into from
Jun 5, 2024
Merged

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented Jun 4, 2024

What?

  • Updates the API for WP_Theme_JSON_Schema_Gutenberg::migrate to accept an origin.
  • Use the new origin argument to conditionally migrate changes in theme.json v3

Why?

See the discussion in https://github.com/WordPress/gutenberg/pull/61262/files#r1589940214

@oandregal

Perhaps we can absorb $config['isGlobalStylesUserThemeJSON'] = true; as part of WP_Theme_JSON_Data_Gutenberg. We may want to still check that's true before returning to be extra-safe (line 553) but we wouldn't need a second transformation.

@joemcgill

Looks like the $config['isGlobalStylesUserThemeJSON'] modification was made as part of 2012a36 (#58409).

@ajlende is there a reason why this would still be necessary if we use the WP_Theme_JSON_Gutenberg object already present in the WP_Theme_JSON_Data_Gutenberg object?

@ajlende

The $config['isGlobalStylesUserThemeJSON'] is required for migrations that don't need to run for the custom origin in WP_Theme_JSON_Schema::migrate (called on new WP_Theme_JSON()). If the origin could be passed to migrate in some other way, then it would be fine.

/*
* Remaining changes do not need to be applied to the custom origin,
* as they should take on the value of the theme origin.
*/
if (
isset( $new['isGlobalStylesUserThemeJSON'] ) &&
true === $new['isGlobalStylesUserThemeJSON']
) {
return $new;
}

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

@ajlende ajlende 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 4, 2024
@ajlende ajlende self-assigned this Jun 4, 2024
Copy link

github-actions bot commented Jun 4, 2024

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/class-wp-theme-json-gutenberg.php
❔ lib/class-wp-theme-json-resolver-gutenberg.php
❔ lib/class-wp-theme-json-schema-gutenberg.php
❔ lib/experimental/kses.php

@joemcgill joemcgill self-requested a review June 4, 2024 21:12
Copy link
Member

@oandregal oandregal left a comment

Choose a reason for hiding this comment

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

I've left a comment to address and it seems this needs a rebase. But it's a really nice update. Thanks for the follow-up!

@oandregal oandregal force-pushed the try/theme-json-migrate-origin branch from 7c796cd to b14c869 Compare June 5, 2024 07:27
@oandregal oandregal marked this pull request as ready for review June 5, 2024 07:28
Copy link

github-actions bot commented Jun 5, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ajlende <[email protected]>
Co-authored-by: oandregal <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@oandregal
Copy link
Member

This is blocked by the performance test failures, which are unrelated and are being investigated at #62331

@oandregal oandregal enabled auto-merge (squash) June 5, 2024 12:29
@oandregal oandregal merged commit 68beef3 into trunk Jun 5, 2024
62 checks passed
@oandregal oandregal deleted the try/theme-json-migrate-origin branch June 5, 2024 13:01
@github-actions github-actions bot added this to the Gutenberg 18.6 milestone Jun 5, 2024
@oandregal
Copy link
Member

Backport at WordPress/wordpress-develop#6737

pento pushed a commit to WordPress/wordpress-develop that referenced this pull request Jun 6, 2024
Backports WordPress/gutenberg#62305

Follow-up to [58328], #61282.

Props ajlende, oandregal, ramonopoly, mukesh27.
Fixes #61282.


git-svn-id: https://develop.svn.wordpress.org/trunk@58354 602fd350-edb4-49c9-b593-d223f7449a82
markjaquith pushed a commit to markjaquith/WordPress that referenced this pull request Jun 6, 2024
Backports WordPress/gutenberg#62305

Follow-up to [58328], #61282.

Props ajlende, oandregal, ramonopoly, mukesh27.
Fixes #61282.

Built from https://develop.svn.wordpress.org/trunk@58354


git-svn-id: http://core.svn.wordpress.org/trunk@57806 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-actions bot pushed a commit to gilzow/wordpress-performance that referenced this pull request Jun 6, 2024
Backports WordPress/gutenberg#62305

Follow-up to [58328], #61282.

Props ajlende, oandregal, ramonopoly, mukesh27.
Fixes #61282.

Built from https://develop.svn.wordpress.org/trunk@58354


git-svn-id: https://core.svn.wordpress.org/trunk@57806 1a063a9b-81f0-0310-95a4-ce76da25c4cd
patil-vipul pushed a commit to patil-vipul/gutenberg that referenced this pull request Jun 17, 2024
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