-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Add style variation classes to rendered body #51512
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
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +19 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
@pbking I just tested this out by manually patching in your work into the latest Gutenberg I appreciate your approach to checking for I think some extenders may ask: "Why is the However, I like this approach that it is only available in the final front-end render. Having conditional CSS (or more) targeting the I believe this would benefit extenders to have this functionality. Would you like to open up this PR and perhaps update it by pulling in the latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
a50fe55
to
99d8b01
Compare
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/global-styles-and-settings.php |
I am in camp "add all the body classes to the iframe body" :) |
/** | ||
* Adds the `uses-style-variation` and `is-style-variation-name` classes to the body if a theme style variation is used. | ||
* | ||
* @since 6.4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tentatively, it would need to be merged today to be in 6.4.
} | ||
return $classes; | ||
} | ||
add_filter( 'body_class', 'gutenberg_add_variation_class_to_body' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For anyone looking into this, the value is escaped later.
…at to add a style variation specific class to the rendered body
a751f07
to
bdc3c2e
Compare
Flaky tests detected in bdc3c2e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6248324610
|
I have a couple of thoughts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a detailed response in the issue, but I don't think we should add this to the body class.
If a developer needs this in the body class, they can add it easily enough per the proposed mod in setUserConfig. It's quite a specific need for a theme (variation) and not useful for the vast majority—or really any active block themes—as far as I'm aware.
@scruffian Consider an extender wanting to utilize JavaScript in their theme to target the body class. |
I can now see the use for this, but I fear that it's encouraging developers to add CSS to their theme rather than in the style variation. As a @richtabor pointed out, themes can already all this themselves so I suggest we leave it to themes to add it when needed and reconsider if we find that most themes do add it. |
I’m on board for just adding the I could see how the style variation slug would make sense. I’m not sure what the benefits/fhallenges would be for variation title or filename. 🤔 This change would require a bit of additional awareness and documentation to make extenders aware of it. |
Yes, I would rather have the change to the setUserConfig than not have the title available at all. |
It doesn't look like this PR is currently ready to be merged, and as the next version of Gutenberg (16.7) will be released today, I'm going to remove the If this should be considered for inclusion in WordPress 6.4, please add the |
Fixes: #43405
What?
Add a mechanism to store what style variation is used and leverage that to add a style variation specific class to the rendered body.
Why?
So that variation-specific CSS can be added to a theme.
How?
When a variation is selected the Title of that variation (defined in the variation's JSON) is stored as a 'custom' Global Styles variable. (This should probably be stored in a better place, but I was mostly ripping out a Proof Of Concept today.)
When the page is rendered if the custom variable is available the classes
uses-style-variation
andis-style-variation-VARIATION-TITLE
are added to thebody
.Testing Instructions
Activate a theme with style variations (Such as Blockbase)
Select a style variation via Global Styles panel and save.
Render any page
Note that the expected classes have been added
Select the default style variation and save.
render any page
note that there are no additional classes