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 filter not rendering in some places #41037

Closed
wants to merge 4 commits into from

Conversation

matiasbenedetto
Copy link
Contributor

@matiasbenedetto matiasbenedetto commented May 12, 2022

What?

Fixing duotone rendering in the pattern selection panel.

Why?

This is necessary to make the pattern preview accurate.

How?

With this change, the duotone presets will render if the block supports duotone and has no custom duotone applied.

Previously we were rendering just the custom duotone applied to the current block but if the block has a filter applied globally using theme.json, the duotone doesn't work because the preset filters are not rendered.

Testing Instructions

  1. Use a theme that supports duotone for example Skatepark
  2. Create a new post or page
  3. Open the add pattern panel
  4. See the patterns that include blocks without a custom duotone applied but with duotone presets in theme.json for example "Full-width image with aside caption" from Skatepark.

Screenshots or screencast

Before: After:
image image
image image

@Mamaduka Mamaduka requested a review from ajlende May 13, 2022 04:37
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.

I don't think we should need to edit duotone.js to fix this. The editor.BlockListBlock hook that you've edited is for rendering custom per-block filters. It is run on every block and should only take the block attributes and render the duotone filters associated with that block.

Did you figure out why some previews were rendering but not others?

@matiasbenedetto
Copy link
Contributor Author

matiasbenedetto commented May 13, 2022

I don't think we should need to edit duotone.js to fix this.

Ok, I'll explore an alternative.

Did you figure out why some previews were rendering but not others?

Yep, the answer is in the PR description:

Previously, we were rendering just the custom duotone applied to the current block, but if the block has a filter applied globally using theme.json, the duotone doesn't work because the preset filters are not rendered.

@ajlende
Copy link
Contributor

ajlende commented May 13, 2022

Did you figure out why some previews were rendering but not others?

Previously, we were rendering just the custom duotone applied to the current block, but if the block has a filter applied globally using theme.json, the duotone doesn't work because the preset filters are not rendered.

Maybe a more apt question would be this: why are all the global styles being rendered in the previews except for the duotone ones?

@matiasbenedetto
Copy link
Contributor Author

matiasbenedetto commented May 23, 2022

I'm closing this PR in favor of this alternative approach: #41249

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants