search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.51k stars 4.2k forks source link

Tracking: Theme.json schema block properties updates #37735

Open ryanwelcher opened 2 years ago

ryanwelcher commented 2 years ago

This issue will track the changes required to address #36630 on a per bock basis.

Each block needs to have its settings property checked to confirm what properties are actually supported in the block. For example, the core/button blocks border setting only supports the radius property so only that property should be added to its entry under the settingsBlocksPropertiesComplete property - see https://github.com/WordPress/gutenberg/pull/36746/files#diff-65c0d061536e5fa3369aba19faf1f475337c549f26828424fa224908f0bcf137R373-R394

Existing Blocks

ryanwelcher commented 2 years ago

36746 addressed the border property for the core/button block

carolinan commented 2 years ago

I am trying to understand how I can contribute to this, can you provide more details? "Each block needs to have its settings property checked to confirm what properties are actually supported in the block."

Can this be automated? Because of course there will be more changes made to the block properties as the experimental features change.

If not. perhaps there should also be a checkbox in the pull request template, to encourage contributors to update the schema when they commit changes to the block.

carolinan commented 2 years ago

Is this the part that needs to be updated? For each block? https://github.com/WordPress/gutenberg/pull/36746/files#diff-65c0d061536e5fa3369aba19faf1f475337c549f26828424fa224908f0bcf137R373-R394

ryanwelcher commented 2 years ago

Can this be automated? Because of course there will be more changes made to the block properties as the experimental features change.

If not. perhaps there should also be a checkbox in the pull request template, to encourage contributors to update the schema when they commit changes to the block.

I'm not sure how much we can automate here, some blocks have explicit entries in their supports object that we can use ( i.e the button block definesborder.radius ) but other settings, such as color, are not as clear to determine if they are supported without setting up a theme.json and seeing what happens.

I have been doing this all manually for now and keeping track of it in gist files for now. This is a large initial effort but once we're done I love the idea of adding a checklist item to update the schema moving forward.

ryanwelcher commented 2 years ago

Is this the part that needs to be updated? For each block? https://github.com/WordPress/gutenberg/pull/36746/files#diff-65c0d061536e5fa3369aba19faf1f475337c549f26828424fa224908f0bcf137R373-R394

Yes! That's the example I'd refer to. Sorry it was not made clear in the issue description what needs to be done.

ryanwelcher commented 2 years ago

@ajlende it looks like some blocks ( i.e core/archives ) don't support any setting overrides at all.

In that case, I am thinking we should keep the reference to the block but add a description to it to indicate that it doesn't support any overrides.

What do you think of omething like the following? The messaging probably need some workshopping but that's the idea

"settingsBlocksPropertiesComplete": {
    "type": "object",
    "properties": {
        "core/archives": {
           "description": "As of WordPress 5.9, this block does not support any settings overrides",
            "additionalProperties": false
        }
   }
}
carolinan commented 2 years ago

I don't think it is 100% clear what a "setting override" is. I mean there is filtering the block, and then there is setting true, false or null values in theme.json, and the latter is not overriding, is it? 🤔

ryanwelcher commented 2 years ago

I don't think it is 100% clear what a "setting override" is.

Maybeblock-level settings?

ryanwelcher commented 2 years ago

I don't think it is 100% clear what a "setting override" is. I mean there is filtering the block, and then there is setting true, false or null values in theme.json, and the latter is not overriding, is it? 🤔

I guess my thought there was that it was overriding the top-level settings object setting in theme.json. i.e settings.border.radius is true but then a block can disabled/override that setting with settings.block.{blockname}.border.radius being set to false

ryanwelcher commented 2 years ago

@carolinan I added the approach above to #37778 so we can iterate there. Thanks for the feedback!

ajlende commented 2 years ago

Can this be automated? Because of course there will be more changes made to the block properties as the experimental features change.

...some blocks have explicit entries in their supports object that we can use ( i.e the button block defines border.radius ) but other settings, such as color, are not as clear to determine if they are supported without setting up a theme.json and seeing what happens.

@carolinan I think it would be a good idea to generate the entire theme.json from source as much as we can. The tsconfig-reference seems to be the most advanced that I've seen in that aspect, so it can be used as a reference for our own automation. They generate some documentation from source, but they have a tsconfigRules.ts that augments and adds additional context to what can be generated from source.

To @ryanwelcher's point, we could maybe generate the things we can from supports and add in the rest with a themeJsonRules.js file.

If not. perhaps there should also be a checkbox in the pull request template, to encourage contributors to update the schema when they commit changes to the block.

I would approve a PR that adds that as an easy first step for now.

ryanwelcher commented 2 years ago

Created #37780 for the checklist update.

ajlende commented 2 years ago

In that case, I am thinking we should keep the reference to the block but add a description to it to indicate that it doesn't support any overrides.

What do you think of something like the following? "As of WordPress 5.9, this block does not support any settings overrides"

I think noting that in the description is better than removing the block entirely. Just a couple suggestions for workshopping the wording.

  1. These descriptions are for the block, so some additional context for what the block is could be useful. Maybe just copied from the block.json description with the note about no properties afterward. This would be easy to do with automation when we get there too.
  2. I think referencing the WordPress version is unnecessary—the schema is already versioned with git tags.

So maybe "Archive block. Display a monthly archive of your posts. This block has no block-level settings." or something?

ryanwelcher commented 2 years ago

So maybe "Archive block. Display a monthly archive of your posts. This block has no block-level settings." or something?

Works for me! I've updated the PR #37778