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 #38055

Merged
merged 1 commit into from
Jan 19, 2022
Merged

Conversation

scruffian
Copy link
Contributor

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.

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 added the [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi label Jan 18, 2022
@scruffian scruffian self-assigned this Jan 18, 2022
@ajlende ajlende requested a review from oandregal January 18, 2022 18:31
@bph
Copy link
Contributor

bph commented Jan 19, 2022

I love the custom color idea for the Duotone. It just feels weird that it would be set via the Customizer interface as it takes the user out of the block editor. Isn't the Customizer going away soon?

@scruffian
Copy link
Contributor Author

Yes ideally this should be possible from within Global Styles. Skatepark is a universal theme so it is attempting to achieve the same things as Global Styles though the customizer, while we transition users to the Site Editor.

@ajlende
Copy link
Contributor

ajlende commented Jan 19, 2022

We also have an issue (#36541) open for adding duotone to global styles, but as far as I know, development hasn't started on that part yet.

Copy link
Contributor

@ajlende ajlende left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like everything is working fine in the themes that usually test

@scruffian scruffian merged commit 74530bc into trunk Jan 19, 2022
@scruffian scruffian deleted the update/duotone-filters branch January 19, 2022 15:45
@github-actions github-actions bot added this to the Gutenberg 12.5 milestone Jan 19, 2022
@@ -158,7 +158,7 @@ function wp_get_global_styles_svg_filters() {

$supports_theme_json = WP_Theme_JSON_Resolver_Gutenberg::theme_has_support();

$origins = array( 'default', 'theme' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, the code in the folder lib/compat/wordpress-5.9 is expected to be part of WordPress 5.9? Is this being ported to WordPress core? I presume it isn't, so we'd need to revert this PR and look at different ways to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert here: #38150

Copy link
Contributor

@ajlende ajlende Jan 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just port to core instead of reverting and working around it here? Is that just more complicated or something?

Copy link
Member

@oandregal oandregal Jan 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, if the change was part of 5.9, this would have been fine. Though when this landed only the 5.9 leads could approve PRs for core and that would need to happen before.

In general, code in a lib/compat/wordpress-X.Y folder needs to be in sync with what was shipped in that same WordPress X.Y release.

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.

4 participants