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

Global Styles: Prevent duplicate CSS for block style variations #6827

Conversation

aaronrobertshaw
Copy link

@aaronrobertshaw aaronrobertshaw commented Jun 14, 2024

This PR backports the PHP updates from: WordPress/gutenberg#62465

It now also includes the missing changes to get_block_nodes from WordPress/gutenberg#41217.

The changes prevent redundant or duplicate CSS being generated for block style variations.

Test Instructions

  1. Register a custom block style variation e.g. through a theme.json partial: /styles/partial.json
{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 2,
	"title": "Custom",
	"slug": "custom",
	"blockTypes": [ "core/group" ],
	"styles": {
		"color": {
			"background": "aliceblue"
		}
	}
}
  1. In both editors add a group block and apply the Custom block style variation.
  2. Inspect the group block and confirm there is only a single is-style-custom-* css class matched
  3. Save the page or post and confirm the same on the frontend

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


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.

Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@aaronrobertshaw
Copy link
Author

I noticed while creating this backport of WordPress/gutenberg#62465 that there's a difference in signature between core and Gutenberg for get_block_nodes.

Before this change:

Core: private static function get_block_nodes( $theme_json )
Gutenberg: private static function get_block_nodes( $theme_json, $selectors = array() )

I'll try and sort this out in time for beta 3.

@aaronrobertshaw
Copy link
Author

aaronrobertshaw commented Jun 16, 2024

The PR modifying the function signature in Gutenberg was WordPress/gutenberg#41217.

From WordPress/gutenberg#41217 (comment) it looks like it the fact the PR wasn't backported was already known however perhaps not that "everything will be fine" as suggested after WordPress/gutenberg#46579.

As far as I can tell, I don't think there'll be any harm in updating get_block_nodes function to match Gutenberg's version. I'll make that update shortly.

@aaronrobertshaw
Copy link
Author

I've made the required updates from WordPress/gutenberg#41217.

Flagging this as ready for review once the Gutenberg PR lands.

@aaronrobertshaw aaronrobertshaw marked this pull request as ready for review June 16, 2024 06:14
Copy link

github-actions bot commented Jun 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.

Core Committers: Use this line as a base for the props when committing in SVN:

Props aaronrobertshaw, mukesh27, ramonopoly, isabel_brison.

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

* @return array The block nodes in theme.json.
*/
private static function get_block_nodes( $theme_json ) {
$selectors = static::get_blocks_metadata();
private static function get_block_nodes( $theme_json, $selectors = array(), $options = array() ) {
Copy link
Member

Choose a reason for hiding this comment

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

I've checked these changes against WordPress/gutenberg#41217 and smoke tested this PR.

LGTM

Thanks for cleaning that up.

@ramonjd
Copy link
Member

ramonjd commented Jun 16, 2024

This PR is testing well for me. For blocks with the block style variation applied, I'm seeing the following, related classnames in the class attribute in both the editor and frontend:

is-style-custom is-style-custom-b321ad1e-3e38-4a3c-af91-ef82f0c9855d

@aaronrobertshaw
Copy link
Author

I'm seeing the following, related classnames in the class attribute in both the editor and frontend

For the moment, this is expected. The is-style-custom classname is what's applied by the editor and is hooked onto by the block support to generate the class specific to this instance or application of the block style, is-style-custom--<hash>.

Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
@tellthemachines
Copy link
Contributor

Just a quick note that applying this patch on top of the latest trunk (after the custom CSS fix) still shows everything working as expected 👍

@oandregal
Copy link
Member

Committed at https://core.trac.wordpress.org/changeset/58422

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.

5 participants