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

Enable transparency manipulation #10958

Closed
wants to merge 1 commit into from
Closed

Enable transparency manipulation #10958

wants to merge 1 commit into from

Conversation

luminaxster
Copy link
Contributor

I ran into trouble when manipulating colors across themes. I use the same primary color(deepOrange) for my light and dark themes, but when trying to blend them with their respective backgrounds it looked messy and required unnecessary coloring logic. Following the same philosophy of lighten/darken, opacify allows blending the palette with the rest of the theme in a simple manner, e.g.: opacify(lighten(theme.palette.primary.light, 0.75), 0.2).

I ran into trouble when manipulating colors across themes. I use the same primary color(deepOrange) for my light and dark themes, but when trying to blend them with their respective backgrounds it looked messy and required unnecessary coloring logic. Following the same philosophy of lighten/darken, opacify allows blending the palette with the rest of the theme in a simple manner, e.g.: opacify(lighten(theme.palette.primary.light, 0.75), 0.2).
@oliviertassinari
Copy link
Member

Don't forget that colorManipulator.js is considered a private module for now: #10789.

@mbrookes
Copy link
Member

mbrookes commented Apr 7, 2018

Am I missing something, or is this not just a copy / paste / rename of fade()?

@oliviertassinari oliviertassinari added the PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI label Apr 7, 2018
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 7, 2018

Am I missing something, or is this not just a copy / paste / rename of fade()?

I confirm. It's the 💯% the same logic. One more good reason to document these helpers. @luminaxster A pull request is welcomed :).

@luminaxster
Copy link
Contributor Author

Oh man, fade went through me when I saw it. Thank you, guys!

@mbrookes
Copy link
Member

mbrookes commented Apr 7, 2018

Yeah, sorry, the name isn't the most intuitive, but it was like that when I found it. 😁

If we're going to document them, now might be the time to rationalise the naming.

@luminaxster luminaxster deleted the patch-1 branch April 10, 2018 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants