-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Backport theme.json version 3 migrations #6616
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
Changes from all commits
668c31e
4c02184
a4e24a0
4696b35
97a24e0
719a4bd
9a0e4b4
915f254
2f9b431
3769e79
dc57d38
943d9bb
0a33eb1
35695f2
7599a7f
ca161da
caaeb72
6569a11
d073251
93e3c82
ea23bf0
afe235e
1b583a1
1e93db4
c8cdcc1
5214e4f
6b6d2b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,7 +39,7 @@ class WP_Theme_JSON_Data { | |
| * @param array $data Array following the theme.json specification. | ||
| * @param string $origin The origin of the data: default, theme, user. | ||
| */ | ||
| public function __construct( $data = array(), $origin = 'theme' ) { | ||
| public function __construct( $data = array( 'version' => WP_Theme_JSON::LATEST_SCHEMA ), $origin = 'theme' ) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this change warrant a
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does it change the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The change in default value here is more to avoid confusion than anything. The same was done in the constructor for The It didn't really matter that much at the time because the migrations weren't changing default values. Since the default value is changed now, keeping this as an empty array means migrating from a v1 theme.json and adding |
||
| $this->origin = $origin; | ||
| $this->theme_json = new WP_Theme_JSON( $data, $this->origin ); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -220,6 +220,7 @@ protected static function has_same_registered_blocks( $origin ) { | |
| * @since 5.8.0 | ||
| * @since 5.9.0 Theme supports have been inlined and the `$theme_support_data` argument removed. | ||
| * @since 6.0.0 Added an `$options` parameter to allow the theme data to be returned without theme supports. | ||
| * @since 6.6.0 Add support for 'default-font-sizes' and 'default-spacing-sizes' theme supports. | ||
| * | ||
| * @param array $deprecated Deprecated. Not used. | ||
| * @param array $options { | ||
|
|
@@ -243,7 +244,7 @@ public static function get_theme_data( $deprecated = array(), $options = array() | |
| $theme_json_data = static::read_json_file( $theme_json_file ); | ||
| $theme_json_data = static::translate( $theme_json_data, $wp_theme->get( 'TextDomain' ) ); | ||
| } else { | ||
| $theme_json_data = array(); | ||
| $theme_json_data = array( 'version' => WP_Theme_JSON::LATEST_SCHEMA ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -310,6 +311,32 @@ public static function get_theme_data( $deprecated = array(), $options = array() | |
| } | ||
| $theme_support_data['settings']['color']['defaultGradients'] = $default_gradients; | ||
|
|
||
| if ( ! isset( $theme_support_data['settings']['typography'] ) ) { | ||
| $theme_support_data['settings']['typography'] = array(); | ||
| } | ||
| $default_font_sizes = false; | ||
| if ( current_theme_supports( 'default-font-sizes' ) ) { | ||
ajlende marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| $default_font_sizes = true; | ||
| } | ||
| if ( ! isset( $theme_support_data['settings']['typography']['fontSizes'] ) ) { | ||
| // If the theme does not have any font sizes, we still want to show the core one. | ||
| $default_font_sizes = true; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't tested, but optional for follow up, these conditions return bools so the return value can be used to set the var, e.g.,
Not really required though, just filling space here... 😄
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The whole thing could use a refactor. I chose to use the same structure as colors, gradients, and shadows above and below this for now to be clear that it was doing exactly the same thing as those other settings. https://github.com/WordPress/wordpress-develop/pull/6616/files#diff-d6b86476eed058e7cf8b6e57fa52c4fd75b1f0907e1a9ccb0149528a24f7578bR294-R302 |
||
| } | ||
| $theme_support_data['settings']['typography']['defaultFontSizes'] = $default_font_sizes; | ||
|
|
||
| if ( ! isset( $theme_support_data['settings']['spacing'] ) ) { | ||
| $theme_support_data['settings']['spacing'] = array(); | ||
| } | ||
| $default_spacing_sizes = false; | ||
| if ( current_theme_supports( 'default-spacing-sizes' ) ) { | ||
| $default_spacing_sizes = true; | ||
| } | ||
| if ( ! isset( $theme_support_data['settings']['spacing']['spacingSizes'] ) ) { | ||
| // If the theme does not have any spacing sizes, we still want to show the core one. | ||
| $default_spacing_sizes = true; | ||
| } | ||
| $theme_support_data['settings']['spacing']['defaultSpacingSizes'] = $default_spacing_sizes; | ||
|
|
||
| if ( ! isset( $theme_support_data['settings']['shadow'] ) ) { | ||
| $theme_support_data['settings']['shadow'] = array(); | ||
| } | ||
|
|
@@ -359,7 +386,7 @@ public static function get_block_data() { | |
| return static::$blocks; | ||
| } | ||
|
|
||
| $config = array( 'version' => 2 ); | ||
| $config = array( 'version' => WP_Theme_JSON::LATEST_SCHEMA ); | ||
| foreach ( $blocks as $block_name => $block_type ) { | ||
| if ( isset( $block_type->supports['__experimentalStyle'] ) ) { | ||
| $config['styles']['blocks'][ $block_name ] = static::remove_json_comments( $block_type->supports['__experimentalStyle'] ); | ||
|
|
@@ -494,6 +521,7 @@ public static function get_user_data_from_wp_global_styles( $theme, $create_post | |
| * Returns the user's origin config. | ||
| * | ||
| * @since 5.9.0 | ||
| * @since 6.6.0 The 'isGlobalStylesUserThemeJSON' flag is left on the user data. | ||
| * | ||
| * @return WP_Theme_JSON Entity that holds styles for user data. | ||
| */ | ||
|
|
@@ -531,14 +559,18 @@ public static function get_user_data() { | |
| isset( $decoded_data['isGlobalStylesUserThemeJSON'] ) && | ||
| $decoded_data['isGlobalStylesUserThemeJSON'] | ||
| ) { | ||
| unset( $decoded_data['isGlobalStylesUserThemeJSON'] ); | ||
| $config = $decoded_data; | ||
| } | ||
| } | ||
|
|
||
| /** This filter is documented in wp-includes/class-wp-theme-json-resolver.php */ | ||
| $theme_json = apply_filters( 'wp_theme_json_data_user', new WP_Theme_JSON_Data( $config, 'custom' ) ); | ||
| static::$user = $theme_json->get_theme_json(); | ||
| $theme_json = apply_filters( 'wp_theme_json_data_user', new WP_Theme_JSON_Data( $config, 'custom' ) ); | ||
| $config = $theme_json->get_data(); | ||
|
|
||
| // Needs to be set for schema migrations of user data. | ||
| $config['isGlobalStylesUserThemeJSON'] = true; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is one of the things I haven't been able to test, and want to leave a note to clarifying it. @ajlende can you provide reproduction steps for this? How does it fail without this?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note this also changed recently at #6271 to remove the extra-parsing.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's used during the migration to prevent adding
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also responded on the Gutenberg PR for #6271. See WordPress/gutenberg#61262 (comment).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Approved the follow-up you prepared WordPress/gutenberg#62305 It's a nice one, thank you!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Backport for that follow-up at #6737 |
||
|
|
||
| static::$user = new WP_Theme_JSON( $config, 'custom' ); | ||
|
|
||
| return static::$user; | ||
| } | ||
|
|
@@ -586,7 +618,6 @@ public static function get_merged_data( $origin = 'custom' ) { | |
| $result = new WP_Theme_JSON(); | ||
| $result->merge( static::get_core_data() ); | ||
| if ( 'default' === $origin ) { | ||
| $result->set_spacing_sizes(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a nice change :) |
||
| return $result; | ||
| } | ||
|
|
||
|
|
@@ -597,12 +628,10 @@ public static function get_merged_data( $origin = 'custom' ) { | |
|
|
||
| $result->merge( static::get_theme_data() ); | ||
| if ( 'theme' === $origin ) { | ||
| $result->set_spacing_sizes(); | ||
| return $result; | ||
| } | ||
|
|
||
| $result->merge( static::get_user_data() ); | ||
| $result->set_spacing_sizes(); | ||
|
|
||
| return $result; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ class WP_Theme_JSON_Schema { | |
| * Function that migrates a given theme.json structure to the last version. | ||
| * | ||
| * @since 5.9.0 | ||
| * @since 6.6.0 Migrate up to v3. | ||
| * | ||
| * @param array $theme_json The structure to migrate. | ||
| * | ||
|
|
@@ -47,8 +48,14 @@ public static function migrate( $theme_json ) { | |
| ); | ||
| } | ||
|
|
||
| if ( 1 === $theme_json['version'] ) { | ||
| $theme_json = self::migrate_v1_to_v2( $theme_json ); | ||
| // Migrate each version in order starting with the current version. | ||
| switch ( $theme_json['version'] ) { | ||
| case 1: | ||
| $theme_json = self::migrate_v1_to_v2( $theme_json ); | ||
| // no break | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these comments required for some linting rule? Otherwise I'd remove them.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like them! But they should probably follow coding standards 😞
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's a standard, I was just thinking that the comment doesn't add much. There's no break. 😄
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if it said
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it could help to stop someone adding a break later on. When I say following coding standards, I mean it should have a capital first letter and end with a period (if the comment is kept).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like it's fine to do in a follow-up?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The linter made me add them. It required the exact string
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, I ran PHPCS in wordpress-develop EDIT: I saved the GB file, but not the WPdev one when I thought it was only in one place. They both show the error.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did some testing and it looks like it only matters that the line // Migrate each version in order starting with the current version.
switch ( $theme_json['version'] ) {
case 1:
$theme_json = self::migrate_v1_to_v2( $theme_json );
// Deliberate fall through. Continue on with migrations.
case 2:
$theme_json = self::migrate_v2_to_v3( $theme_json );
} |
||
| case 2: | ||
| $theme_json = self::migrate_v2_to_v3( $theme_json ); | ||
| // no break | ||
| } | ||
|
|
||
| return $theme_json; | ||
|
|
@@ -84,6 +91,88 @@ private static function migrate_v1_to_v2( $old ) { | |
| return $new; | ||
| } | ||
|
|
||
| /** | ||
| * Migrates from v2 to v3. | ||
| * | ||
| * - Sets settings.typography.defaultFontSizes to false. | ||
| * | ||
| * @since 6.6.0 | ||
| * | ||
| * @param array $old Data to migrate. | ||
| * | ||
| * @return array Data with defaultFontSizes set to false. | ||
| */ | ||
| private static function migrate_v2_to_v3( $old ) { | ||
| // Copy everything. | ||
| $new = $old; | ||
|
|
||
| // Set the new version. | ||
| $new['version'] = 3; | ||
|
|
||
| /* | ||
| * 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; | ||
| } | ||
|
|
||
| /* | ||
| * Even though defaultFontSizes and defaultSpacingSizes are new | ||
| * settings, we need to migrate them as they each control | ||
| * PRESETS_METADATA prevent_override values which were previously | ||
| * hardcoded to false. This only needs to happen when the theme provides | ||
| * fontSizes or spacingSizes as they could match the default ones and | ||
| * affect the generated CSS. | ||
| */ | ||
| if ( isset( $old['settings']['typography']['fontSizes'] ) ) { | ||
| if ( ! isset( $new['settings'] ) ) { | ||
| $new['settings'] = array(); | ||
| } | ||
| if ( ! isset( $new['settings']['typography'] ) ) { | ||
| $new['settings']['typography'] = array(); | ||
| } | ||
|
Comment on lines
+132
to
+137
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be missing something, but this seems unnecessary considering Not a big issue though, I don't mind cautious code. (edit: There's also (edit again: Even better, do what Ramon says - https://github.com/WordPress/wordpress-develop/pull/6616/files/f8e247d7812644d67e946a8f98e734e795bcb9c9#r1625538035) |
||
| $new['settings']['typography']['defaultFontSizes'] = false; | ||
| } | ||
|
|
||
| /* | ||
| * Similarly to defaultFontSizes, we need to migrate defaultSpacingSizes | ||
| * as it controls the PRESETS_METADATA prevent_override which was | ||
| * previously hardcoded to false. This only needs to happen when the | ||
| * theme provided spacing sizes via spacingSizes or spacingScale. | ||
| */ | ||
| if ( | ||
| isset( $old['settings']['spacing']['spacingSizes'] ) || | ||
| isset( $old['settings']['spacing']['spacingScale'] ) | ||
| ) { | ||
| if ( ! isset( $new['settings'] ) ) { | ||
| $new['settings'] = array(); | ||
| } | ||
| if ( ! isset( $new['settings']['spacing'] ) ) { | ||
| $new['settings']['spacing'] = array(); | ||
| } | ||
| $new['settings']['spacing']['defaultSpacingSizes'] = false; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As with the defaultFontSizes above, in PHP you can set the value of a nested array item: $new = [];
$new['settings']['spacing']['defaultSpacingSizes'] = false;
var_dump( $new );
/*
Output:
array(1) {
["settings"]=>
array(1) {
["spacing"]=>
array(1) {
["defaultSpacingSizes"]=>
bool(false)
}
}
}*/
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's follow-up with this. |
||
| } | ||
|
|
||
| /* | ||
| * In v3 spacingSizes is merged with the generated spacingScale sizes | ||
| * instead of completely replacing them. The v3 behavior is what was | ||
| * documented for the v2 schema, but the code never actually did work | ||
| * that way. Instead of surprising users with a behavior change two | ||
| * years after the fact at the same time as a v3 update is introduced, | ||
| * we'll continue using the "bugged" behavior for v2 themes. And treat | ||
| * the "bug fix" as a breaking change for v3. | ||
| */ | ||
| if ( isset( $old['settings']['spacing']['spacingSizes'] ) ) { | ||
| unset( $new['settings']['spacing']['spacingScale'] ); | ||
| } | ||
|
|
||
| return $new; | ||
| } | ||
|
|
||
| /** | ||
| * Processes the settings subtree. | ||
| * | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.