theme.json schema: Allow object type on style properties #45897
Merged
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
mikachan
added
the
Developer Experience
|
|
mikachan
commented
Nov 18, 2022
jffng
reviewed
Nov 18, 2022
|
Thanks for doing this.
I think refs are valid for all properties. |
mikachan
changed the title
mikachan
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
mikachan
commented
Nov 30, 2022
| @@ -145,10 +145,10 @@ Border styles. | |||
|
|
|||
| | Property | Type | Props | | |||
| | --- | --- |--- | | |||
| | color | string | | | |||
| | color | undefined | | | |||
There was a problem hiding this comment.
These undefined types are the result of running npm run docs:build. It looks like these changes need to match the result of this script when run on GitHub, so I've pushed these changes for now. However, this should probably be improved, as undefined isn't that helpful.
I'll spin up a follow-up PR if I figure this out!
There was a problem hiding this comment.
There isn't anything special going on with the docgen—just reading in the json file and formatting a few things as markdown. So it shouldn't be too hard to update the docgen to aggregate the types in the oneOf list which would also fix the radius property.
Eventually it would be nice to generate the schema from a higher-level source that is more suited for docgen. There's a discussion open about that, but I'd like to see the quick fix before tackling the larger problem.
There was a problem hiding this comment.
Thanks @ajlende, that's super helpful context!
There was a problem hiding this comment.
Created a PR for this here: #46375
Thanks, I thought so! I've added the new type to all of the style properties. |
# Conflicts: # docs/reference-guides/theme-json-reference/theme-json-living.md
|
Size Change: 0 B Total Size: 1.32 MB ℹ️ View Unchanged
|
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.
What?
This PR adds support for objects, in addition to strings, for style properties in theme.json.
Do we need to add support for objects on other properties, in addition to color?I've now added support for all style properties.Why?
In #41696, we added the ability to reference values from other locations in the tree. However, currently, this isn't valid markup because the reference is an object rather than a string.
Fixes #45889.
How?
Add
objectto the supported types for style properties, and add a definition forref, calledrefComplete.Testing Instructions
Add the following markup to a theme.json file:
Without this PR, you should see a warning in your text editor that the ref is an incorrect type (
Incorrect type. Expected "string".). With this PR, this warning shouldn't show, at least for style values.cc. @WordPress/block-themers