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

Fix duotone on fixed and repeated cover backgrounds #34021

Closed

Conversation

ajlende
Copy link
Contributor

@ajlende ajlende commented Aug 11, 2021

Description

Fixes #31662

The duotone filter should work with fixed and repeated backgrounds.

For duotone to be applied to an element, that element cannot contain child elements or the duotone will also be applied to the children, so the block markup had to change to accommodate that by having a separate div for the background to apply the duotone to.

How has this been tested?

  1. Add a cover block
  2. Set one of the duotone colors combinations
  3. In the block settings, toggle on "Fixed Background" under Media Settings
  4. In the block settings, toggle on "Repeated Background" under Media Settings

Since this is changing the saved markup, make sure deprecations work by creating a block prior to applying this PR and then upgrading the block.

Screenshots

TODO

Types of changes

Breaking change for the generated cover block markup.

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).

@ajlende ajlende changed the title WIP TODO fix media position on save and update deprecated Fix duotone on fixed and repeated cover backgrounds Aug 11, 2021
@ajlende ajlende added the [Block] Cover Affects the Cover Block - used to display content laid over a background image label Aug 11, 2021
@ntsekouras
Copy link
Contributor

👋 - Do you think the changes in this PR:#33927 would help resolving this duotone issue?

@ajlende
Copy link
Contributor Author

ajlende commented Aug 11, 2021

Yeah, that PR is actually the reason I switched over to this quick—we can batch the changes together so we only need to add one deprecation. These changes and your changes should be more-or-less compatible, and we can merge them together once they're both ready to go.

@ntsekouras
Copy link
Contributor

we can batch the changes together so we only need to add one deprecation. These changes and your changes should be more-or-less compatible, and we can merge them together once they're both ready to go.

Sounds good! I'll work from tomorrow to resolve my remaining issues and start checking your PR as well. Thanks!

@ntsekouras
Copy link
Contributor

It seems my PR will require some adjustments in the approach taken, so I don't want this PR to be blocked. Feel free to move forward independently 😄

@Mamaduka
Copy link
Member

@ajlende. Can we close this PR? The mentioned issue was fixed via #40554.

@ajlende ajlende closed this Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duotone doesn't work with fixed background
3 participants