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

Do not add unregistered style variations to the theme.json schema #49807

Merged
merged 6 commits into from
Apr 18, 2023

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Apr 13, 2023

Follow-up to #46343

What?

This PR makes sure unregistered block style variations are not part of the internal theme.json schema, so they are ignored before processing any data coming from theme.json.

Why?

  • The current state prevents the site editor from loading when an unregistered style variation is found in a theme.json dataset.
  • It also enqueues CSS rules to the front-end that don't have a selector, so they are invalid.

How?

By making sure the style variation is not added to the allowed list, unless it's registered for that particular block.

Testing Instructions

  • Make sure your environment logs the errors to the screen by setting WP_DEBUG to true.
  • Add the following two variations to the theme.json of TwentyTwentyThree:
{
  "styles": {
    "blocks": {
      "core/quote": {
        "variations": {
          "plain": { "color": { "background": "indigo", "text": "gold" } },
          "unregistered": { "color": { "background": "red", "text": "blue" } },
        } 
      }
    }
  }
}
  • Create a post that contains a quote and style it with the plain variation.

The expected result is that:

  • The styles are applied to quote blocks with plain style variation in editors and front.
  • The site editor loads properly.
  • The front-end doesn't have a style rule such as { background-color: red; color: blue} (note the lack of CSS selector).
  • There is no error reported in class-wp-theme-json-gutenberg.php. In trunk, you'll see something like this:

image

Verify that upon updating the colors of the style variation via the global styles sidebar (blocks > quote > style variation plain), the user-provided colors are rendered everywhere (front & editors).

Note that by adding an unregistered variation, the core class class-wp-theme-json.php will still throw an error. This is expected. It'll be fixed when this PR is backported to core.

@oandregal oandregal self-assigned this Apr 13, 2023
@oandregal oandregal added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Apr 13, 2023
@oandregal oandregal changed the title Do not add unregister style variations to the theme.json schema Do not add unregistered style variations to the theme.json schema Apr 13, 2023
Copy link
Contributor

@tellthemachines tellthemachines 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 addressing this! I can partially reproduce the issue on trunk with empty theme, though I can't see the styles for the unregistered variation being output anywhere. I do see the PHP warning and the site editor doesn't load.

In this branch, the warning you mention goes away, and the site editor loads, but I see another warning in the post editor:
Warning: Undefined array key "unregistered" in /var/www/html/wp-includes/class-wp-theme-json.php on line 2219

Also editing existing style variations no longer works (see comment below)

lib/class-wp-theme-json-gutenberg.php Show resolved Hide resolved
@oandregal oandregal marked this pull request as ready for review April 14, 2023 08:55
@oandregal
Copy link
Member Author

@tellthemachines ok, this should be ready now. Updated the issue description with better test instructions and other notes. Hope this helps!

@github-actions
Copy link

Flaky tests detected in 41f50bb.
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/4722396270
📝 Reported issues:

Copy link
Contributor

@tellthemachines tellthemachines left a comment

Choose a reason for hiding this comment

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

Re-testing confirms all is now working correctly. Thanks for adding a test! Code lgtm ✅

@oandregal oandregal merged commit 8d4bbb5 into trunk Apr 18, 2023
@oandregal oandregal deleted the fix/schema-valid-variations branch April 18, 2023 06:41
@github-actions github-actions bot added this to the Gutenberg 15.7 milestone Apr 18, 2023
@colorful-tones
Copy link
Member

@oandregal would this fix the fact that I'm seeing "Property variations not allowed." in VS Code for v2 of the theme.json schema when running WP 6.2 stable? Just wondering if I should open another separate issue for that. Thanks!

Screenshot 2023-04-18 at 4 34 41 PM

@oandregal
Copy link
Member Author

@colorful-tones no, that's a different thing. I see @tellthemachines declared the variations in the schema file in the PR that introduced the feature. I don't use this feature myself, neither I am very familiar with it, so I can't tell if anything else is needed. Perhaps @ajlende knows.

@tellthemachines
Copy link
Contributor

@colorful-tones I can't reproduce that locally on VSCode 🤔
Could your VSCode settings be pointing to an older version of the schema?

@colorful-tones
Copy link
Member

@tellthemachines Sorry for the delay. I believe it is likely that I was not testing against the latest trunk for Gutenberg plugin. I honestly do not have the bandwidth to test and confirm. So, I would classify it as a non-issue for now. Thanks!

@oandregal
Copy link
Member Author

Backport PR at WordPress/wordpress-develop#4554

@oandregal oandregal added the Needs PHP backport Needs PHP backport to Core label Jun 6, 2023
@ramonjd ramonjd removed the Needs PHP backport Needs PHP backport to Core label Jun 7, 2023
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] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants