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

Skatepark: refactor duotone blocks to use the new opacity setting #4493

Closed
wants to merge 1 commit into from

Conversation

MaggieCabrera
Copy link
Contributor

Changes proposed in this Pull Request:

This PR applies the changes from this change to the duotone filter. This changes the block patterns and templates that use duotone so that they don't have the green/black combination but instead they use a black/transparent combination to blend with the background. This means that when we change colors for the theme the duotone images are no longer green, but they blend in with the background.

Screenshot 2021-08-30 at 15-45-21 Duotone tests – fdsfsdf

There's still an issue with the Blue/Cream and Green/Pink color options, since in the design those are expecting the opposite combination of background/foreground colors on their duotone filter. These two combinations are not blending into the background color but instead are doing the opposite:

Screenshot 2021-08-30 at 15-45-47 Duotone tests – fdsfsdf

@MaggieCabrera MaggieCabrera requested review from melchoyce and a team August 30, 2021 13:53
@MaggieCabrera MaggieCabrera added the [Theme] Skatepark Automatically generated label for Skatepark. label Aug 30, 2021
@melchoyce
Copy link
Contributor

What are our technical options for the dark schemes?

@MaggieCabrera
Copy link
Contributor Author

What are our technical options for the dark schemes?

with what we have right now I don't think we can provide a good solution for them. If/when WordPress/gutenberg#34073 lands, we can use variables for the duotone settings and we could potentially swap the values of the variables when we select the darker color schemes.

@kjellr
Copy link
Contributor

kjellr commented Aug 31, 2021

@MaggieCabrera Is this basically a halfway step (which works for lighter colors only) until WordPress/gutenberg#33905 is resolved? While I appreciate that it's better than what we have, it still results in a subpar experience for some users.

Edit: Oh wait, I guess it sounds like WordPress/gutenberg#34073 would basically solve WordPress/gutenberg#33905? Either way, I'm not sure this transparency support is all that consequential... it makes things a little better for right now, but it also doesn't quite solve the problem.

@MaggieCabrera
Copy link
Contributor Author

@MaggieCabrera Is this basically a halfway step (which works for lighter colors only) until WordPress/gutenberg#33905 is resolved? While I appreciate that it's better than what we have, it still results in a subpar experience for some users.

Edit: Oh wait, I guess it sounds like WordPress/gutenberg#34073 would basically solve WordPress/gutenberg#33905? Either way, I'm not sure this transparency support is all that consequential... it makes things a little better for right now, but it also doesn't quite solve the problem.

yeah I agree, it's not a proper solution, but I wanted to explore and share what the transparency PR was bringing anyway, hopefully, we won't need this one. I also wanted to bring attention to the problem with the darker colors, which will definitely need a fix on the theme to switch the duotone color pairs.

@melchoyce
Copy link
Contributor

Thanks for exploring this, @MaggieCabrera, and finding those issues. Let's backburner this while y'all and the Gutenberg team flesh out the feature in core.

@MaggieCabrera
Copy link
Contributor Author

Closing this one in favor of the work done over at WordPress/gutenberg#34667

@MaggieCabrera MaggieCabrera deleted the skatepark-duotone-opacity branch April 18, 2022 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Theme] Skatepark Automatically generated label for Skatepark.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants