Migrate theme.json v1 to v2 #36155
Merged
Migrate theme.json v1 to v2 #36155
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oandregal
added
[Status] In Progress
oandregal
requested review from
aaronrobertshaw,
ockham,
youknowriad and
jorgefilipecosta
This was referenced Nov 2, 2021
There was a problem hiding this comment.
Thanks for all your efforts on the theme.json v2 schema 👍
I appreciate this PR is still in a draft state with a few things left to do but I took it for a spin all the same.
✅ Theme.json doc updates make sense
✅ Unneeded custom prefixes have been removed from class-wp-theme-json-gutenberg.php
✅ New v1 to v2 migration looks good
✅ Updates to global styles and theme.json match
✅ All theme.json related tests are passing
While testing I found a few extra updates we might wish to add to the to-do list if they aren't already.
- Update the Create Block Theme doc
- Update theme.json under the schemas package
- This was only added a couple of days ago in Add schemas package #35998
- This schema package landed after the previous removal of
customprefixes so we'll need to update all those properties.
Also, do we want to provide the same backwards compatibility bridge in useSetting as the other properties that had their custom prefix removed? Or, will that be removed itself soon enough?
Let me know if I can assist in updating the block supports etc that use the renamed theme.json flags. I'm happy to help wherever I can.
oandregal
force-pushed
the
add/theme-json-v2
branch
from
a92edc3 to
a42946e
Compare
oandregal
requested review from
ajitbohra,
ellatrix,
spacedmonkey and
TimothyBJacobs
as code owners
|
Size Change: 0 B Total Size: 1.08 MB
ℹ️ View Unchanged
|
|
@aaronrobertshaw thanks for the early review! I've pushed all changes left (schema, tutorial, client) and updated the issue description with test instructions. This is ready for a final review. |
oandregal
changed the title
| /** | ||
| * Class that implements a migration from a v1 data structure into a v2 one. | ||
| */ | ||
| class WP_Theme_JSON_Schema_V1_To_V2 implements WP_Theme_JSON_Schema { |
There was a problem hiding this comment.
I think one day we will need a v2 to v3 migration and v3 to v4...
Having a file "lib/class-wp-theme-json-schema-v1-to-v2.php" and class per migration "WP_Theme_JSON_Schema_V1_To_V2" does not scale well. Files and classes are part of the public API.
Could we have a single migrate class that then has private methods inside per migration we need?
| /** | ||
| * Class that implements a migration from a v1 data structure into a v2 one. | ||
| */ | ||
| class WP_Theme_JSON_Schema_V1_To_V2 implements WP_Theme_JSON_Schema { |
There was a problem hiding this comment.
Classes should have _Gutenberg suffix otherwise when we pass the class with the same name to core we are going to have a crash.
jorgefilipecosta
approved these changes
Nov 3, 2021
|
I'm going to merge this and try refactoring this to a single class as a follow-up, as per Jorge's suggestion. |
|
Prepared #36182 to have a single class with all migrations. |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #34349
Built on top of #36154
This PR implements v1 to v2 migrations: we only need to remove the
customprefix from some existing properties.Backward compatibility
useSetting( 'spacing.customPadding' )will still work, although the recommended way is usingspacing.padding.Test theme.json v1 still works
Using the following theme.json (for example, with the empty theme):
{ "version": 1, "settings": { "spacing": { "customPadding": false, "customMargin": false }, "typography": { "customLineHeight": false }, "layout": { "contentSize": "840px", "wideSize": "1100px" }, "blocks": { "core/button": { "border": { "customRadius": false } } } } }Verify that:
Set the above properties to
trueand verify that the UI controls are enabled.Test theme.json v2
Using the following theme.json (for example, with the empty theme):
{ "version": 2, "settings": { "spacing": { "padding": false, "margin": false }, "typography": { "lineHeight": false }, "layout": { "contentSize": "840px", "wideSize": "1100px" }, "blocks": { "core/button": { "border": { "radius": false } } } } }Verify that:
Set the above properties to
trueand verify that the UI controls are enabled.