Skip to content
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

Update theme.json schema to allow for per-block management of settings. #36746

Conversation

ryanwelcher
Copy link
Contributor

@ryanwelcher ryanwelcher commented Nov 22, 2021

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/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).

@ryanwelcher
Copy link
Contributor Author

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.

@skorasaurus skorasaurus added the Developer Experience Ideas about improving block and theme developer experience label Dec 29, 2021
Copy link
Contributor

@ajlende ajlende left a comment

Choose a reason for hiding this comment

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.

@ryanwelcher
Copy link
Contributor Author

ryanwelcher commented Jan 5, 2022

@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

@ajlende
Copy link
Contributor

ajlende commented Jan 5, 2022

Looks like the docgen for the theme.json broke because of the structure change. Care if I push a few commits to this branch to fix it? Otherwise, the change would be in https://github.com/WordPress/gutenberg/blob/trunk/bin/api-docs/gen-theme-reference.js.

@ryanwelcher
Copy link
Contributor Author

Looks like the docgen for the theme.json broke because of the structure change. Care if I push a few commits to this branch to fix it? Otherwise, the change would be in https://github.com/WordPress/gutenberg/blob/trunk/bin/api-docs/gen-theme-reference.js.

Not at all! I just merged #37729 as well so be sure to pull latest trunk

@ajlende
Copy link
Contributor

ajlende commented Jan 5, 2022

Not at all! I just merged #37729 as well so be sure to pull latest trunk

Cool, will do!

@ajlende ajlende force-pushed the feature/update-theme-json-schema-36630 branch from f45716a to 396d9cf Compare January 5, 2022 19:14
@ajlende
Copy link
Contributor

ajlende commented Jan 5, 2022

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.

@ryanwelcher ryanwelcher merged commit 90c1e7d into WordPress:trunk Jan 6, 2022
@github-actions github-actions bot added this to the Gutenberg 12.4 milestone Jan 6, 2022
@ajlende ajlende added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Jan 28, 2022
@adamziel adamziel removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants