-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Duotone: Backport from Core to fix filters in classic themes #54778
Conversation
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/class-wp-duotone-gutenberg.php |
lib/class-wp-duotone-gutenberg.php
Outdated
wp_add_inline_style( $style_tag_id, self::get_global_styles_presets( self::$used_global_styles_presets ) ); | ||
} | ||
if ( ! empty( self::$block_css_declarations ) ) { | ||
wp_add_inline_style( $style_tag_id, wp_style_engine_get_stylesheet_from_css_rules( self::$block_css_declarations ) ); |
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.
Should the prefix of wp_style_engine_get_stylesheet_from_css_rules()
function be changed to gutenberg_
?
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.
Should the prefix of
wp_style_engine_get_stylesheet_from_css_rules()
function be changed togutenberg_
?
Yeah, the gutenberg_
prefixed functions should be preferred over the wp_
ones if they exist in the plugin. This way we're testing the changes to those functions everywhere within the plugin. That way issues are much more likely to surface before porting things to core.
Hello ! Side question : now that duotone is part of block rendering, and no more in global styles, how do we force the ouput of SVGs to user Duotone outside the content of a post. I was just applying Thx ! |
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
b5c1e1f
to
d633540
Compare
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.
Thanks @t-hamano! Sorry for the delay on the review—I got really busy for a while and forgot about this one.
I've rebased with trunk, made the gutenberg_
prefix change, and tested the whole thing. It looks good to me. 👍
Fixes #54099
Related Core PR: WordPress/wordpress-develop#4839
What?
This PR applies the Duotone filter fix in the Classic theme committed in Core to the Gutenberg plugin.
Why?
Without this PR, the Dutone filter will not be applied to Classic themes when the Gutenberg plugin is activated, as reported in #54099.
How?
I applied the changes made in the core as is.
Testing Instructions
Verify that the image block duotone filter works in both block and classic themes.