-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Behaviors - Lightbox: Update theme.json schema #51156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this 🙂 I just have one comment:
It seems that when defining "settings.blocks.core/image.behaviors.lightbox` still complains in Visual Studio:
I am not familiar with schemas but, if we are using allOf in the code (here), should we include "behaviors" here as well? If I am not mistaken, that would allow other blocks to include the "behaviors" property, but that is something that is already happening. For example, I can define "settings.blocks.core/post-date.border" which isn't supported.
I guess we could try using anyOf as well, but that seems to allow any property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess there's a need to be specific until the behavior API is extended to more blocks. I think we could still create a behaviors properties definition, similar to settingsPropertiesComplete
et al so that our future selves, and other devs, know where to add them.
It seems that when defining "settings.blocks.core/image.behaviors.lightbox` still complains in Visual Studio:
Is your theme's theme.json pointing to "$schema": "https://schemas.wp.org/trunk/theme.json",
?
You can point it to the local copy. See the README
schemas/json/theme.json
Outdated
@@ -487,7 +487,7 @@ | |||
"type": "string" | |||
}, | |||
"fluid": { | |||
"description": "Specifics the minimum and maximum font size value of a fluid font size. Set to `false` to bypass fluid calculations and use the static `size` value.", | |||
"description": "Specifies the minimum and maximum font size value of a fluid font size. Set to `false` to bypass fluid calculations and use the static `size` value.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, good one! I spotted this yesterday and fixed. 👍
I have tested with the following file, which references this PR as the schema URL. {
"$schema": "https://raw.githubusercontent.com/WordPress/gutenberg/update/theme-json-schema-behaviors/schemas/json/theme.json",
"version": 2
} The description of the root level This is because the default type of value is an object, even though it is described as "an array of blocks." How about moving this description to It would be nice to have validation in the The The |
I was testing it pointing to a local file and making changes to it seems to work 🤷 Thank you anyway for the information! 🙂 |
f8cc3d2
to
a9e379f
Compare
I've updated the PR with the correct types for the Lightbox animations. I think this is ready now, but we should wait until removing the experimental flag as mentioned by Carlos. Just noting here that we'll also need to change the types for |
What?
Updating the theme.json schema with the new values available for Behaviors UI and the Lightbox feature.
As Behaviors UI and the Lightbox feature can be enabled and disabled through the theme.json file. Updating the schemas should facilitate its use for Theme developers.
Important:⚠️ ⚠️ Do not merge while behaviors and lightbox are behind the experimental flag.