Skip to content

Ensure that custom block element styles in theme.json are not ignored#4174

Closed
felixarntz wants to merge 6 commits into
WordPress:trunkfrom
felixarntz:fix/57868
Closed

Ensure that custom block element styles in theme.json are not ignored#4174
felixarntz wants to merge 6 commits into
WordPress:trunkfrom
felixarntz:fix/57868

Conversation

@felixarntz

Copy link
Copy Markdown
Member
  • Add a new private function to centralize lookup logic based on theme.json path.
  • Add test coverage, both an actual unit test for the new function and integration tests as part of wp_add_global_styles_for_blocks().
  • Fix lookup logic to include custom block element styles and make it more resilient, as the block name should always be at a specific position.

Trac ticket: https://core.trac.wordpress.org/ticket/57868


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@felixarntz

Copy link
Copy Markdown
Member Author

@glendaviesnz @scruffian Would be great to get your reviews on this since you worked on the original code in WordPress/gutenberg#41446.

I worked on test coverage first. Seeing them fail on 308066b but pass on d1dcd9d shows that the updated logic correctly fixes the underlying bug.

@felixarntz felixarntz marked this pull request as ready for review March 6, 2023 01:09
Comment thread src/wp-includes/global-styles-and-settings.php
Comment thread src/wp-includes/global-styles-and-settings.php Outdated
Comment thread src/wp-includes/global-styles-and-settings.php
@glendaviesnz

Copy link
Copy Markdown

This tested well for me. I added:

	"styles": {
		"blocks": {
			"core/quote": {
				"elements": {
					"cite": {
						"color": {
							"text": "blue",
							"background": "yellow"
						}
					}
				}
                         }
			"coblocks/accordion": {
				"elements": {
					"h2": {
						"color": {
							"text": "red",
							"background": "black"
						}
					}
				}
			}

and without this patch element styles for the core and custom block displayed in the editor, but only the core block element style showed in the frontend. With this patch applied both element styles displayed in the editor and frontend.

Thanks for looking at this.

Comment thread src/wp-includes/global-styles-and-settings.php Outdated
Comment thread src/wp-includes/global-styles-and-settings.php Outdated
Comment thread tests/phpunit/tests/theme/wpAddGlobalStylesForBlocks.php
Comment thread tests/phpunit/tests/theme/wpAddGlobalStylesForBlocks.php
@felixarntz felixarntz requested review from audrasjb, azaozz, costdev, oandregal and spacedmonkey and removed request for costdev July 17, 2023 22:20
Comment thread src/wp-includes/global-styles-and-settings.php

@azaozz azaozz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@costdev costdev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the updates @felixarntz! LGTM 👍

@felixarntz

Copy link
Copy Markdown
Member Author

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.

6 participants