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: Don't allow css property except at the root and block level #53156

Closed
wants to merge 1 commit into from

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Jul 30, 2023

Fixes #52217

What?

This PR changes the theme.json schema to not allow the insertion of CSS properties except at the root level of the styles property and directly under styles.blocks.

Why?

For now, css property are only available in the site root and block and will not work if specified on variations or elements. It would make sense to display a warning according to the schema when a CSS property is defined that does not work.

How?

Removed css property from stylesProperties . This is because if this property is present, it will appear as a candidate, even if the css property is not allowed.

candidate

Removed css property from stylesElementsPropertiesComplete and stylesPropertiesComplete. This will trigger a warning when css properties are defined primarily at the element level.

Instead of removing css from stylesProperties, add description and type to css properties in styles and stylesPropertiesAndElementsComplete. This is to display a description when inserting a property and to insert a default empty string when the property is inserted.

description

Testing Instructions

Create locally a theme.json with the schema directed to this PR:

theme.json sample
{
	"$schema": "https://raw.githubusercontent.com/WordPress/gutenberg/schema/css-only-block-level/schemas/json/theme.json",
	"version": 2,
	"styles": {
		"css": "",
		"blocks": {
			"core/button": {
				"css": "",
				"elements": {
					"button": {
						"css": "",
						":hover": {
							"css": ""
						}
					},
					"link": {
						"css": "",
						":hover": {
							"css": ""
						}
					}
				},
				"variations": {
					"wide": {
						"css": ""
					}
				}
			},
			"my/custom": {
				"css": "",
				"elements": {
					"css": "",
					"button": {
						":hover": {
							"css": ""
						}
					}
				},
				"variations": {
					"wide": {
						"css": ""
					}
				}
			}
		},
		"elements": {
			"button": {
				"css": "",
				":hover": {
					"css": ""
				}
			},
			"link": {
				"css": "",
				":hover": {
					"css": ""
				}
			}
		}
	}
}

Use this theme.json file to confirm the following:

  • When typing "css" under styles and styles.blocks.{blockname}, a property with a description should appear in the candidate list. If you add a definition, you should not get a warning.
  • If you type "css" anywhere else, it should not appear as a candidate. If you defined the property manually, you should get a warning.

@t-hamano t-hamano changed the title theme.json shcema: Don't allow css property except at the block level theme.json shcema: Don't allow css property except at the root and block level Jul 30, 2023
@t-hamano t-hamano self-assigned this Jul 30, 2023
@t-hamano t-hamano added [Type] Developer Documentation Documentation for developers Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Jul 30, 2023
@github-actions
Copy link

Flaky tests detected in 62e3e13.
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/5706409803
📝 Reported issues:

Comment on lines -286 to -325
---

### css

Sets custom CSS to apply styling not covered by other theme.json properties.


Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the schema is strictly defined, the css property will unfortunately be removed from the document. It may be possible to revive this description by slightly forcing the docs:theme-ref logic to be adjusted.

However, now that css properties are only allowed in certain hierarchies, I personally think it is acceptable. If in the future, css properties will be available everywhere related to style, I think it would be fine to add them at that time.

@t-hamano t-hamano marked this pull request as ready for review July 30, 2023 13:49
@bph bph changed the title theme.json shcema: Don't allow css property except at the root and block level theme.json schema: Don't allow css property except at the root and block level Jul 31, 2023
@t-hamano t-hamano force-pushed the schema/css-only-block-level branch from c369508 to 2c85d09 Compare March 2, 2024 11:31
Copy link

github-actions bot commented Mar 2, 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 <[email protected]>
Co-authored-by: carolinan <[email protected]>

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

@t-hamano
Copy link
Contributor Author

For now, css property are only available in the site root and block and will not work if specified on variations or elements. It would make sense to display a warning according to the schema when a CSS property is defined that does not work.

I tested this again and it seems the css works on both elements and variations. Perhaps it's related to the section styles introduced in WP6.6.

Therefore, I'm closing this PR.

@t-hamano t-hamano closed this Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Developer Documentation Documentation for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theme.json schema: Remove CSS from block style variations and elements
1 participant