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

Theme JSON schema: Fix "not allowed error" in settings property #54521

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Sep 16, 2023

Related to #54509

What?

This PR fixes an issue in the theme.json schema that causes errors where all properties in the settings property are not allowed except ligntbox.

Before After
before after

Why?

#54509 added a reference to the Lightbox schema (settingsPropertiesLightbox) to the settings property. settingsPropertiesLightbox defines additionalProperties :true at the root level of the object, so this applies to the settings property itself. As a result, properties other than the lightbox property are no longer allowed.

How?

Moved additionalProperties inside the lightbox property. This will solve the problem with the settings property, and will also output errors for properties that are not allowed in the lightbox property. My guess is that this is expected behavior.

Testing Instructions

Create a json file like the one below locally:

{
	"$schema": "https://raw.githubusercontent.com/WordPress/gutenberg/schema/fix-settings-definition/schemas/json/theme.json",
	"version": 2,
	"settings": {
		"lightbox": {
		}
	}
}
  • Confirm that no error occurs when you enter properties other than the lightbox property directly under settings.
  • Confirm that an error occurs when you enter properties other than allowEditing and enabled in the settings.lightbox property.

@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Type] Developer Documentation Documentation for developers [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. labels Sep 16, 2023
@t-hamano t-hamano self-assigned this Sep 16, 2023
@github-actions
Copy link

Warning: Type of PR label error

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Accessibility (a11y), [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Performance, [Type] Project Management, [Type] Regression, [Type] Security, [Type] WP Core Ticket, Backport from WordPress Core.
  • Labels found: [Type] Bug, [Type] Developer Documentation, [Feature] Themes.

Read more about Type labels in Gutenberg.

@t-hamano t-hamano marked this pull request as ready for review September 16, 2023 09:01
Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

LGTM and fixes the issue 👍

@t-hamano
Copy link
Contributor Author

@aristath Thanks for the review 🚀

Copy link
Member

@walbo walbo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@t-hamano t-hamano changed the title Theme JSON schema: Fix "not allowd error" in settings property Theme JSON schema: Fix "not allowed error" in settings property Sep 18, 2023
@t-hamano t-hamano merged commit 6db51a6 into trunk Sep 18, 2023
50 of 57 checks passed
@t-hamano t-hamano deleted the schema/fix-settings-definition branch September 18, 2023 09:17
@github-actions github-actions bot added this to the Gutenberg 16.7 milestone Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants