-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[system] Add blend color manipulator #40258
Conversation
Netlify deploy previewhttps://deploy-preview-40258--material-ui.netlify.app/ Bundle size reportDetails of bundle changes (Toolpad) |
c7fb7db
to
005338d
Compare
* @param {number} [gamma=1.0] - Gamma correction factor. For gamma-correct blending, 2.2 is usual. | ||
*/ | ||
export function blend(background, overlay, opacity, gamma = 1.0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally set the gamma correction factor to 2.2, but I had to set it back to 1.0 because that's how browsers do it and blending wouldn't match browser behavior otherwise.
It's just another case in a long serie of Everyone Does sRGB Wrong Because Everyone Else Does sRGB Wrong
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add few tests to make sure we won't break it in the future.
I have addressed the comments & added tests. |
@mnajdova Is this one good for merging? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay. I am merging it.
|
||
describe('blend', () => { | ||
it('works', () => { | ||
expect(blend('rgb(90, 90, 90)', 'rgb(10, 100, 255)', 0.5)).to.equal('rgb(50, 95, 173)'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, it's pretty much what I get with:
background: color-mix(in srgb, rgb(90, 90, 90), rgb(10, 100, 255) 50%);
It's not supported by https://mui.com/material-ui/getting-started/supported-platforms/#browser but browser support it pretty good today: https://caniuse.com/?search=color-mix.
* @param {number} opacity - Opacity multiplier in the range 0 - 1 | ||
* @param {number} [gamma=1.0] - Gamma correction factor. For gamma-correct blending, 2.2 is usual. | ||
*/ | ||
export function blend(background, overlay, opacity, gamma = 1.0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we export this as
export function blend(background, overlay, opacity, gamma = 1.0) { | |
export function unstable_blend(background, overlay, opacity, gamma = 1.0) { |
?
With #40958, assuming we can use color-mix, I'm not clear why we need to make this util part of the public API. cc @DiegoAndai
Add a blend function to
colorManipulator.js
, which is used in mui/mui-x#10059.