[colorManipulator] Inconsistent output values with the `darken` function

[arronhunt]

Current behavior 😯

When using MUI's internal darken function (material-ui/packages/mui-system/src/colorManipulator.js) we see very different color outputs depending on the format of the input color value. For example:

/* All these values are the same color and written in different formats */
let hex = "#7B61FF";
let rgb = "rgb(123, 97, 255)";
let hsl = "hsl(250, 100%, 69%)";

let coefficient = 0.1; // Can be any value

darken(hex, coefficient) // returns rgb(110, 87, 229)
darken(rgb, coefficient) // returns rgb(110, 87, 229)
darken(hsl, coefficient) // returns hsl(250, 100%, 62.1%) which is very different than the first two

Preview (Left column is inputs, right column is outputs):

This is due to the way that the darken function manipulates colors. If it detects an hsl color, it will lower the lightness by the coefficient. However, if it is RGB (or is cast to RGB) the function will adjust each separate color component by the coefficient. The latter is a more naïve approach to darkening since the result is usually desaturated.

Expected behavior 🤔

Color adjustments should output a consistent color value regardless of input format. TinyColor's darken function is a good example; it always casts an input value to HSL and then makes the adjustment, outputting the same results for every color format.

Steps to reproduce 🕹

Live example: https://codesandbox.io/s/magical-dubinsky-zrehcj?file=/src/index.js

Context 🔦

This is causing issues with keeping our design files in sync with our Figma mockups because not all color values are in the same base color format.

Your environment 🌎

No response

[Mark-777-0]

@siriwatknp

Would you like me to standardize the darken function ?

I'd be glad to help!

[siriwatknp]

@siriwatknp

Would you like me to standardize the darken function ?

I'd be glad to help!

Feel free to open a PR. If it is not a breaking change, we can move forward and merge it.

[arronhunt]

@Mark-777-0 In my ticket description I referenced TinyColor’s method which converts all colors to HSL before transforming them. IMHO this is the most optimal way to adjust a color’s “darkness.” because it retains the original hue and saturation levels intended by the designer. In contrast, the RGB method is more akin to overlaying pure black with an alpha value which results in some loss of saturation.

Here’s a great article that goes deeper into detail https://sujansundareswaran.com/blog/why-hsl-is-better-than-hex-and-rgb

[Mark-777-0]

Hi @siriwatknp I have begun editing the darken function, but while setting up my environment I've noticed that instead of a /src directory there are /legacy, /modern and /esm files within the node_modules and @mui/systems. After a bit of time I figured out that editing the esm folder is what allows me to modify the darken function.

Should I add something about this in the contributing docs https://github.com/mui/material-ui/blob/master/CONTRIBUTING.md , maybe something is already there but I might have missed it. I'd be glad to add a section on how to set up a local environment, as it might save future contributors some time. Should I create a separate issue for this?

[Mark-777-0]

@Mark-777-0 In my ticket description I referenced TinyColor’s method which converts all colors to HSL before transforming them. IMHO this is the most optimal way to adjust a color’s “darkness.” because it retains the original hue and saturation levels intended by the designer. In contrast, the RGB method is more akin to overlaying pure black with an alpha value which results in some loss of saturation.

Here’s a great article that goes deeper into detail https://sujansundareswaran.com/blog/why-hsl-is-better-than-hex-and-rgb

Thank you for this! Let me give it a shot

[Mark-777-0]

@arronhunt hopefully I didn't mess anything up and you can use it soon!

Edit: Looks like I messed something up

[arronhunt]

@Mark-777-0 looks like a few tests failing, want to collaborate and see if we can get this over the finish line? :)

[arronhunt]

@siriwatknp can you clarify why this is tagged "new feature" and not "bug" ?

[siriwatknp]

@siriwatknp can you clarify why this is tagged "new feature" and not "bug" ?

My mistake.

[1CUNHA1]

hey, is this still open for fixing?

[arronhunt]

@1CUNHA1 there's a pending PR for it #40050