You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or 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
This PR is an attempt at addressing #36630. It separates out each of the setting objects ( border, color, spacing etc ) into individual definitions and then combines them on a per-block basis using allOf. This allows the schema to use any or all of them to create a schema that is both valid and also reflects what the block actually supports.
I have includes a single updated block in this PR so we can discuss if this is the correct approach. The core/button block only implements the controls for the border.radius option so I have updated its entry to reflect such.
How has this been tested?
Tested locally by pointing the $schema entry to the theme.json schema definition in this PR
Types of changes
Schema updates that should have no breaking affect on blocks.
Checklist:
My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
I've tested my changes with keyboard and screen readers.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
If this approach seems sound, it may make sense to merge this and then create a tracking ticket for the other blocks. I have spent a fair amount of time looking into the per-block support and can handle the creation of that ticket. It would also be a great place for new contributors to get some props.
The reason will be displayed to describe this comment to others. Learn more.
Sorry it took me so long to get around to reviewing this, but I like the idea! Let's get the merge conflicts resolved and do like you said with creating a tracking issue to update the rest of the blocks.
@ajlende I have addressed the merge conflicts but am blocked by some e2e tests. The navigation block e2e failures are unrelated to the changes here. Based on @youknowriad 's comment here I have also created a PR to skip the test with #37729
Alright, I pushed a quick fix that will generate the same docs as before. As a follow-up when all the blocks are updated we can update the docgen to show the settings per-block.
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.
skorasaurus
added
the
Developer Experience
ryanwelcher
requested review from
adamsilverstein,
ajitbohra,
cameronvoell,
dmsnell,
ellatrix,
gziolo,
nerrad and
ntwb
as code owners
ajlende
force-pushed
the
feature/update-theme-json-schema-36630
branch
from
adamziel
removed
the
Backport to WP Minor Release
Description
This PR is an attempt at addressing #36630. It separates out each of the setting objects ( border, color, spacing etc ) into individual definitions and then combines them on a per-block basis using
allOf. This allows the schema to use any or all of them to create a schema that is both valid and also reflects what the block actually supports.I have includes a single updated block in this PR so we can discuss if this is the correct approach. The
core/buttonblock only implements the controls for theborder.radiusoption so I have updated its entry to reflect such.How has this been tested?
Tested locally by pointing the
$schemaentry to the theme.json schema definition in this PRTypes of changes
Schema updates that should have no breaking affect on blocks.
Checklist:
*.native.jsfiles for terms that need renaming or removal).