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

Add JSON Schema for WordPress block.json files #1879

Merged

Conversation

fabiankaegy
Copy link
Contributor

@fabiankaegy fabiankaegy commented Oct 13, 2021

This PR adds a JSON Schema for the block.json file used in WordPress development to define metadata for blocks for the editor.
The reference this schema is based on can be found here: https://developer.wordpress.org/block-editor/reference-guides/block-api/block-metadata/

Closes #1878

@fabiankaegy
Copy link
Contributor Author

@ajlende since you did the work on #1874 I'd love to get your thoughts on the addition of the block.json schema I added here. I think it is now ready for review but I would love your feedback before actually marking this as "Ready for review" and therefore subject to get merged.

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.

This is great! I'm very happy that other folks are taking this schema thing and running with it for WordPress. 🙂

I noted a few of my personal opinions about styling for schemas. A lot of my inspiration for theme-v1.json came from the tsconfig.json schema since that was a popular project that I was familiar with. If we have a better way to do thing, we should do it the better way—I'm not very strongly opinionated on the things that I mentioned.

The one thing that I do think needs to be changed before merging is removing __experimentalDuotone. More notes about that in the inline comment.

src/schemas/json/block.json Outdated Show resolved Hide resolved
src/schemas/json/block.json Outdated Show resolved Hide resolved
src/schemas/json/block.json Outdated Show resolved Hide resolved
src/schemas/json/block.json Outdated Show resolved Hide resolved
src/schemas/json/block.json Outdated Show resolved Hide resolved
@fabiankaegy
Copy link
Contributor Author

@ajlende Thank you so much for your thorough review. I have taken your feedback and fixed the different suggestions. I wasn't able to cut down all the descriptions to the 2 sentence limit since I think in the cases where they go past that it is because it shares valuable information.

@fabiankaegy fabiankaegy marked this pull request as ready for review October 15, 2021 07:31
@fabiankaegy fabiankaegy changed the title Draft: Add WordPress block.json schema Add WordPress block.json schema Oct 15, 2021
@fabiankaegy fabiankaegy changed the title Add WordPress block.json schema Add JSON Schema for WordPress block.json files Oct 15, 2021
@ajlende
Copy link
Contributor

ajlende commented Oct 15, 2021

I love it! Thanks for your work on this! ❤️

@madskristensen
Copy link
Contributor

Let me know when ready to merge

@fabiankaegy
Copy link
Contributor Author

@madskristensen We are :)

@ajlende
Copy link
Contributor

ajlende commented Oct 15, 2021

################ Error message
>> compile    | schemas/json/block.json (draft-04)
>> Error: strict mode: unknown keyword: "deprecated"
##############################

For this error, we may want to either note that it's deprecated in the description or remove the property altogether. I'd rather stick to draft-04 than updating to 2019-09 which adds deprecated keyword because draft-04 is more widely supported in general and because the WordPress JSON Schema validator (currently only used for REST APIs AFAIK) only supports draft-04 as well.

That being said, the markdownDescription property that is used in the tsconfig isn't part of draft-04 either, and it looks like we can add exceptions in https://github.com/SchemaStore/schemastore/blob/4608f1f42725b22ad4c82462a09b16c6ad8eb60f/src/schema-validation.json if we want to go that route.

@fabiankaegy
Copy link
Contributor Author

I just removed the deprecated property since the description already has it written in it.

@madskristensen So it is ready to be merged :)

@madskristensen madskristensen merged commit 8f71ddd into SchemaStore:master Oct 18, 2021
@madskristensen
Copy link
Contributor

Thanks

mkaz added a commit to WordPress/gutenberg that referenced this pull request Oct 21, 2021
Updates documentation recommending to use schema validation.

A block.json JSON schema was added to the SchemaStore that allows for
editors to provide additional tooltip, autocomplete, and some validation
when working with a block.json file.

Added in: SchemaStore/schemastore#1879
mkaz added a commit to WordPress/gutenberg that referenced this pull request Oct 21, 2021
Updates documentation recommending to use schema validation.

A block.json JSON schema was added to the SchemaStore that allows for
editors to provide additional tooltip, autocomplete, and some validation
when working with a block.json file.

Added in: SchemaStore/schemastore#1879
"items": {
"type": "string",
"enum": [
"vertical"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, according to the PR that introduced support for this in Gutenberg, there are a couple of options to declare support for padding at https://github.com/WordPress/gutenberg/pull/33859/files#diff-cb2052b95e3c5b5fa711f6519d1e3c376597cd1747e6a0a781dc51447255ef16R588

  • boolean
  • individual properties: an array that can contain any of top, left, bottom, right
  • axial properties: an array that can contain any of vertical, horizontal

From what I've gathered in this PR I presume that we don't need two items, one for vertical here and the other for horizontal below, but, instead, we need to declare the enum like [ 'vertical', 'horizontal' ]. Does that make sense? It seems this is creating issues, see WordPress/gutenberg#35902 (comment)

Same for the margin declaration.

cc @fabiankaegy @ajlende

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 #1903

Copy link
Contributor Author

@fabiankaegy fabiankaegy Oct 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that. I misread the following line

A spacing property may support arbitrary individual sides or axial sides, but not a mix of both.

from the handbook to mean that it can only have either or of the axial controls but rereading it that was incorrect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add WordPress block.json schema
4 participants