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

JSON schema: Update schema for background support #59127

Merged
merged 2 commits into from
Feb 19, 2024

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Feb 16, 2024

Follow up #53934 and #57005

What?

This PR adds and updates background support definitions in theme.json and block.json.

Why?

To enhance the developer experience.

How?

I've updated the description to reflect the actual implementation, but one point that confused me is that backgroundSize actually covers three things: background-size, background-position, and background-repeat. That's why I mentioned it in the description.

Testing Instructions

  • Create a JSON file locally using the test data below.
  • Make sure the code editor properly indicates suggestions, default values, and description.

theme.json

{
	"$schema": "https://raw.githubusercontent.com/WordPress/gutenberg/schema/add-background-image/schemas/json/theme.json",
	"version": 2,
	"settings": {
	}
}
theme.json.mp4

block.json

{
	"$schema": "https://raw.githubusercontent.com/WordPress/gutenberg/schema/add-background-image/schemas/json/block.json",
	"version": "2",
	"name": "test/test",
	"title": "test",
	"supports": {
	}
}
block.json.mp4

@t-hamano t-hamano added the [Type] Developer Documentation Documentation for developers label Feb 16, 2024
@t-hamano t-hamano self-assigned this Feb 16, 2024
@t-hamano t-hamano marked this pull request as ready for review February 16, 2024 15:25
Copy link

github-actions bot commented Feb 16, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this up @t-hamano, and for the great testing instructions, that made it easy to double-check that it's working correctly!

image image image

but one point that confused me is that backgroundSize actually covers three things: background-size, background-position, and background-repeat. That's why I mentioned it in the description.

Yes, it isn't obvious at first, but during work on the features it became apparent that repeat and position controls cannot reasonably exist separately to backgroundSize, so it would be better for the features to be consolidated in terms of settings, into the catch-all of backgroundSize.

If we wanted to make this grouping clearer, we could perhaps adjust the wording in the schema to something like Allow blocks to define values related to the size of a background image, including size, position, and repeat controls.

That said, I like the wording you've gone with, so this LGTM, too! ✨

@t-hamano
Copy link
Contributor Author

@andrewserong Thanks for the review!

If we wanted to make this grouping clearer, we could perhaps adjust the wording in the schema to something like Allow blocks to define values related to the size of a background image, including size, position, and repeat controls.

I also feel that this is more clear 👍

theme.json:

image

block.json:

image

@andrewserong
Copy link
Contributor

Nice tweaks there, that reads well to me!

Copy link

github-actions bot commented Feb 19, 2024

Flaky tests detected in f53f565.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7953100288
📝 Reported issues:

@t-hamano t-hamano changed the title JSON schema: Updated schema for background support JSON schema: Update schema for background support Feb 19, 2024
@t-hamano t-hamano merged commit c6e08e0 into trunk Feb 19, 2024
56 checks passed
@t-hamano t-hamano deleted the schema/add-background-image branch February 19, 2024 01:57
@github-actions github-actions bot added this to the Gutenberg 17.8 milestone Feb 19, 2024
@t-hamano
Copy link
Contributor Author

Oh sorry, I forgot to update the prop...

@andrewserong
Copy link
Contributor

No worries! 😄

@carolinan
Copy link
Contributor

carolinan commented Mar 25, 2024

Isn't this feature enabled by default? Should it be true, not false?
Update: nevermind, of course I forgot to delete appearancetools when I was testing something :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants