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

Duotone: Allow users to specify custom filters #38416

Closed
wants to merge 3 commits into from
Closed

Conversation

scruffian
Copy link
Contributor

@scruffian scruffian commented Feb 1, 2022

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

Screenshot 2022-01-18 at 16 30 00

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

@scruffian scruffian requested review from oandregal and ajlende and removed request for spacedmonkey, ajitbohra and TimothyBJacobs February 1, 2022 16:18
@scruffian scruffian self-assigned this Feb 1, 2022
@scruffian scruffian added the [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi label Feb 1, 2022
*
* @return string
*/
function wp_get_global_styles_svg_filters() {
Copy link
Member

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.

Copy link
Member

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.

Copy link
Contributor Author

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 ?

Copy link
Member

@oandregal oandregal Feb 2, 2022

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:

Appreciate your patience here, @scruffian and @ajlende Sorry to have you go round in circles.

@oandregal oandregal requested a review from gziolo February 1, 2022 16:32
@scruffian
Copy link
Contributor Author

Closing this in favour of #38442

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants