-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: Allow users to specify custom filters #38416
Conversation
* | ||
* @return string | ||
*/ | ||
function wp_get_global_styles_svg_filters() { |
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 see that the wp_get_global_styles_svg_filters
is not present in WordPress 5.9 but still lives in the lib/compat/5.9
folder of Gutenberg. Apparently, #36236 didn't make the cut for 5.9 and is slated for the next minor release (it has the "backport to WP minor release" label).
My understanding of how we use the lib/compat
folder is that we should move the existing wp_get_global_styles_svg_filters
to a lib/compat/5.9.1
folder. Once there, we can make the changes introduced by this PR directly, as it wasn't shipped in a WordPress version yet.
cc @gziolo and @youknowriad for confirmation.
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'm not sure if we need to be so much specific about the minor version because we remove code in batch for a major version line. The existing lib/compat/5.9
directory should be fine.
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.
So are you saying we didn't need to revert #38055 ?
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.
Ah, I see. For efficiency, I asked for further clarifications in private about what happens with PRs that need to go on a minor and this is what I've learned:
- they should be labeled with "Backport to WP minor"
- they go in the folder of the major version, in this case,
lib/compat/5.9
We did use a 5.8.1 folder in the past but it's not clear that was super useful.
There's the issue of what happens with code that was supposed to go in a minor but a minor didn't happen: it seems that we need to port that code to the folder of the next major (lib/compat/6.0
in this case) at some point (I presume we do this review when we try to remove the compat
folder, but I haven't been involved in this process, to be honest).
So, the way forward to incorporate this feature would be:
- make sure the dependency is marked for a minor Fix duotone theme cache #36236 (done)
- go with the 1st attempt Duotone: Allow users to specify custom filters #38055 and label it for minor as well
Appreciate your patience here, @scruffian and @ajlende Sorry to have you go round in circles.
Closing this in favour of #38442 |
Description
In order to allow themes to specify their own custom duotone filters, we need to allow SVG to be output for filters defined in the custom key of the Global Styles json object.
This is an alternative to #38055. I'm not sure if this approach is right.
How has this been tested?
Use the Skatepark theme
Open the customizer and select a custom color
Check that images have duotone applied with the new colors
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.js
files for terms that need renaming or removal).