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

Add new filter or API to allow modifying theme.json elements' style selectors #59163

Closed
daviedR opened this issue Feb 19, 2024 · 7 comments
Closed
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Enhancement A suggestion for improvement.

Comments

@daviedR
Copy link
Contributor

daviedR commented Feb 19, 2024

What problem does this address?

When defining element styles in the theme.json or customizing the Global Styles, WP will generate a global CSS for the specified element.

For example, if a theme define a button element style in the theme.json like this:

...
"styles": {
	"button": {
		"typography": {
			"fontWeight": "600"
		}
	}
}
...

WP will generate this global CSS:

.wp-element-button, .wp-block-button__link { font-weight: 600 }

The valid elements list (and the selectors) are defined in the class-wp-theme-json.php file (ELEMENTS constant). It's also defined in the JS file: package/blocks/src/api/constants.js in the __EXPERIMENTAL_ELEMENTS constant. These are double definitions, one for PHP and the other for JS. The JS constant is used in Site Editor mode to generate real-time global CSS from the Global Styles settings panel.

The valid elements list and the selectors are currently fixed and can't be modified using filters.

In a few cases, theme developers might want to add new selectors to the valid elements list. For example, I want to add .button into the button element selectors.

Original selector: wp-element-button, .wp-block-button__link
It will become: wp-element-button, .wp-block-button__link, .button

When using the :hover pseudo selector, the selector should be: .wp-element-button:hover, .wp-block-button__link:hover, .button:hover.


Currently, there is no way to modify the selectors properly and implement this kind of feature.

I did some workaround on the PHP side by using the wp_theme_json_get_style_nodes filter to check each style node and add the new selectors manually. It seems more like a hack than a proper filter. The snippet is like this:

add_filter(
	'wp_theme_json_get_style_nodes',
	function( $nodes ) {
		foreach ( $nodes as &$node ) {
			if ( array( 'styles', 'elements', 'button' ) === $node['path'] ) {
				if ( -1 < strpos( $node['selector'], ':hover' ) ) {
					// Add selector for button's style node with :hover pseudo selector.
					$node['selector'] = $node['selector'] . ', .button:hover';
				} else {
					// Add selector for button's style node.
					$node['selector'] = $node['selector'] . ', .button';
				}
			}
		}

		return $nodes;
	}
);

That works perfectly both in the front end and Post Editor because the CSS is loaded via PHP.

But this doesn't work in the Site Editor. The Site Editor JS will always replace the global styles with the newly generated CSS according to the Global Styles settings panel. The CSS selector used in the newly generated CSS is the one defined in the __EXPERIMENTAL_ELEMENTS constant which is hard coded and can't be modified.


Also, there's also a minor bug in the JS side __EXPERIMENTAL_ELEMENTS constant.
The selectors for the link element are different from the PHP one.
in PHP: a:where(:not(.wp-element-button))
in JS: a

What is your proposed solution?

I think there should be a new filter or API to modify the element's selectors and make it synced between PHP and JS.

@aaronrobertshaw
Copy link
Contributor

Thanks for taking the time to write up such a nicely detailed issue @daviedR 👍

In a few cases, theme developers might want to add new selectors to the valid elements list. For example, I want to add .button into the button element selectors.

Could you expand a little on the use case that is driving the desire to add .button to element selectors?

Also, I think there is a minor typo in the example theme.json snippet should it be styles.elements.button rather than styles.button?

Also, there's also a minor bug in the JS side __EXPERIMENTAL_ELEMENTS constant.

Good catch! I recently fixed an issue where the elements block support used that same simple a selector on the frontend leading to buttons incorrectly getting a custom link style's color. I completely missed the selector in the editor though.

For the record, the simpler selector in the site editor doesn't appear to have any negative consequences there as the button block renders as a div in the editor rather than the a on the frontend.

To make things consistent though, in case it helps prevent future issues or confusion, I've thrown up a quick PR to tweak that selector: #59167

@jordesign jordesign added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Feb 21, 2024
@daviedR
Copy link
Contributor Author

daviedR commented Feb 22, 2024

Thanks for your response @aaronrobertshaw

Could you expand a little on the use case that is driving the desire to add .button to element selectors?

It's not specifically .button, it can be many other selectors for other elements.

The idea is to allow developers to add more selectors to element styles, so whenever users change the styles in theme.json or via Global Styles panel, it will reflect to the added selectors as well.


Another example of use cases for this:

As a theme developer, I want to provide a shortcut on the core/paragraph block to replicate the h1, h2, h3, h4, h5, or h6 styles as defined in theme.json or Global Styles panel.

I think block styles are the proper way for this. So I added 6 block styles on the core/paragraph block: h1, h2, h3, h4, h5, and h6.

Note: The user actually could replicate h1, h2, h3, h4, h5, and h6 styles manually by changing the Typography settings on the block. But the idea with this block style is that the styles would always sync with the Global Styles or theme.json changes.

image

When the h3 block style is chosen by the user, WP will add .is-style-h3 CSS class to the block. This should turn the paragraph styles into the h3 styles.

I think the only proper way to do this is to modify the element selectors and add the .is-style-h3 to the styles.elements.h3 styles. So whatever changes in the theme.json or Styles Variation .json files or Global Styles panel, they will reflect in the .is-style-h3 CSS class.

I managed to do this using the wp_theme_json_get_style_nodes PHP filter and add the selectors. It works perfectly on the Post Editor screen.

But it doesn't work in the Global Styles panel (Site Editor), because it uses a JS with hard-coded element selectors (__EXPERIMENTAL_ELEMENTS) to generate the real-time CSS. I can't add my new selectors to it.


By the way, thanks for the PR #59167 👍

@aaronrobertshaw
Copy link
Contributor

The extra details help greatly, thanks!

It's not specifically .button, it can be many other selectors for other elements.

As you've noted previously, the element selectors are used in some different places. I'm not 100% convinced of the value in filtering those selectors when weighed against the potential for unexpected consequences etc. Particularly, if there might be some other options to solve the problem.

Your idea around leveraging block style variations might hold some promise on that front!

I'm actually doing some work on extending block style variations that might help here, over in #57537 & #57908. One of the goals there is to allow theme.json and global style configuration of custom block style variations.

When the h3 block style is chosen by the user, WP will add .is-style-h3 CSS class to the block. This should turn the paragraph styles into the h3 styles.

From what I understand of the use case, this is the key wrinkle you're trying to solve, how to keep the custom paragraph variations in sync with user customizations in Global Styles?

This sounds a lot like the reasoning behind ref values in theme.json as touched on in the Global Settings & Styles docs under "Referencing a style". Using a reference within a block style variation (post #57908), could look something like:

{
    ...
    "styles": {
        "blocks": {
            "core/paragraph": {
                "variations": {
                    "h3": {
                        "color": {
                            "text": { "ref": "styles.elements.h3.color.text" },
                        },
                        "typography": {
                            "fontSize": { "ref": "styles.elements.h3.typography.fontSize" },
                        },
                    }
                }
            }
        }
    }
}

From the above example, when the global styles stylesheet is generated, along with the block style variation styles, the value used will be sourced from the referenced location. That would include any customizations made by the user in Global Styles.

The current work around block style variations isn't set in stone so we can still refine it in light of further real-world use cases. Given all that, I think there's a good chance the new API you're chasing could actually be the enhanced block style variations.

It would also likely be available much sooner than new filters around element selectors, as I'm hoping that feature will land in Gutenberg early in the 6.6 release cycle. You can following along on those efforts via the main tracking issue.

If you'd like to help speed things along, the more people we can get testing the overall reduction of global block style specificity, that underpins the feature, the better.

Of course, if I've completely missed the mark here and misunderstood the use case, or there are others not addressed via block style variations, please don't hesitate to share them.

@daviedR
Copy link
Contributor Author

daviedR commented Mar 5, 2024

Thanks for your reply!

I agree with you, adding a new filter to modify the element selectors might have unexpected consequences. In this case, using the ref value in the theme.json seems to be a better option.

After testing on using the ref value in theme.json for my use case above, it works perfectly on front website, post editor, and even the site editor (real time CSS when customizing Global Styles).

However, in the block style variation definition, we need to declare all CSS properties we want to sync.

For example, here is our theme.json:

{
    ...
    "styles": {
        "elements": {
            "h3": {
                "typography": {
                    "fontSize": "2rem"
                }
            }
        },
        "blocks": {
            "core/paragraph": {
                "variations": {
                    "h3": {
                        "typography": {
                            "fontFamily": { "ref": "styles.elements.h3.typography.fontFamily" },
                            "fontSize": { "ref": "styles.elements.h3.typography.fontSize" },
                            "fontStyle": { "ref": "styles.elements.h3.typography.fontStyle" },
                            "fontWeight": { "ref": "styles.elements.h3.typography.fontWeight" },
                            "letterSpacing": { "ref": "styles.elements.h3.typography.letterSpacing" },
                            "lineHeight": { "ref": "styles.elements.h3.typography.lineHeight" },
                            "textDecoration": { "ref": "styles.elements.h3.typography.textDecoration" },
                            "textTransform": { "ref": "styles.elements.h3.typography.textTransform" }
                        }
                    }
                }
            },
            "core/post-title": {
                "variations": {
                    "h3": {
                        "typography": {
                            "fontFamily": { "ref": "styles.elements.h3.typography.fontFamily" },
                            "fontSize": { "ref": "styles.elements.h3.typography.fontSize" },
                            "fontStyle": { "ref": "styles.elements.h3.typography.fontStyle" },
                            "fontWeight": { "ref": "styles.elements.h3.typography.fontWeight" },
                            "letterSpacing": { "ref": "styles.elements.h3.typography.letterSpacing" },
                            "lineHeight": { "ref": "styles.elements.h3.typography.lineHeight" },
                            "textDecoration": { "ref": "styles.elements.h3.typography.textDecoration" },
                            "textTransform": { "ref": "styles.elements.h3.typography.textTransform" }
                        }
                    }
                }
            }
        }
    }
}

Although we only define the fontSize property for our h3 element, we need to apply ref to all typography properties.

Without all those references, if we change h3 style other than fontSize in the Global Styles (for example, fontWeight), the style won't be applied to the h3 style variation of other blocks.

This is of course not a bug or an issue, just a side note on how to use ref value for my use case.


Note: I also found a few bugs while implementing this ref value into my use case. I've created separate tickets for each bug:

  1. theme.json block style variation doesn't generate CSS for a new block style registered via register_block_style() function #59584
  2. Invalid real time generated CSS for ref value when changing font sizes presets in the Global Styles panel (Site Editor) #59585

I found another bug:

The block style generated CSS for the core/paragraph block has invalid selector: .is-style-h3.is-style-h3p.
It should be: p.is-style-h3.
This bug happened on WordPress 6.4.3 without Gutenberg plugin.
But I noticed that this has been fixed when Gutenberg plugin (v17.8.2) is activated, so I didn't create a ticket for this.

@aaronrobertshaw
Copy link
Contributor

Thanks for taking the time to create issues when you encounter them @daviedR 👍

I've left a comment regarding #59584 about where you can follow related progress.

When I have the bandwidth, I'll also look into #59585.

The block style generated CSS for the core/paragraph block has invalid selector

FYI this was fixed in #58051 and backported to WP Core via WordPress/wordpress-develop#6053. In other words, the fix will be released in WP 6.5.

@aaronrobertshaw
Copy link
Contributor

@daviedR with a fix up for the invalid CSS value for a ref style and the invalid selector issue already fixed in 6.5 / Gutenberg, would you have any objections to closing this issue?

@daviedR
Copy link
Contributor Author

daviedR commented Mar 14, 2024

Hi @aaronrobertshaw
Thanks for all your help in this. I will close this issue.

@daviedR daviedR closed this as completed Mar 14, 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] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

3 participants