cal.com
cal.com copied to clipboard
[CAL-1603] Replace hacky color functions in getBrandColor.ts
Currently all the functions here are written from scratch
There is a library which makes this file a lot easier to read and not as complicated if we ever need to debug it.
From SyncLinear.com | CAL-1603
I want to work on this issue plz assign this to me
Hi @sean-brydon , I would love to work on this issue. My approach to solving the issue will be as follows:
- Importing the package in getBrandedColors.tsx file
- Replacing the functions written from scratch with functions provided by
polished
package in the colors sections (link) as follows
function rgbToHex(r: number, g: number, b: number): string // can be replaced to ⬇️
rgb(value: (RgbColor | number) , green: number?, blue: number?): string
// the rgb function provided by the package takes RGB values as params and returns hex valued string
-
Similarly, the package has functions to
lighten
anddarken
the color with respect to amount provided along with the RGB color -
The package has a function
getLuminance
which returns the luminescence as used in the code multiple times. -
The code has a function - checkWCAGContrastColor() which first finds the contrast color based on 2 colors passed as strings. This finding of contrast ration can be replaced by getContrast(color1: string, color2:string): number function.
Apart from above functions, I felt that other functions are core functions and can't be replaced by the library functions. If there are any, then I will surely replace as you say!
Can you assign this issue to me. Thank you!
Hi @sean-brydon , I would love to work on this issue. My approach to solving the issue will be as follows:
- Importing the package in getBrandedColors.tsx file
- Replacing the functions written from scratch with functions provided by
polished
package in the colors sections (link) as followsfunction rgbToHex(r: number, g: number, b: number): string // can be replaced to ⬇️ rgb(value: (RgbColor | number) , green: number?, blue: number?): string // the rgb function provided by the package takes RGB values as params and returns hex valued string
- Similarly, the package has functions to
lighten
anddarken
the color with respect to amount provided along with the RGB color- The package has a function
getLuminance
which returns the luminescence as used in the code multiple times.- The code has a function - checkWCAGContrastColor() which first finds the contrast color based on 2 colors passed as strings. This finding of contrast ration can be replaced by getContrast(color1: string, color2:string): number function.
Apart from above functions, I felt that other functions are core functions and can't be replaced by the library functions. If there are any, then I will surely replace as you say!
Can you assign this issue to me. Thank you!
Thanks @SohamRatnaparkhi - I've passed this over to you. The lighten/darken/rgb were the ones i was on about. Nice spot on the getContrast too!
Please give me a shout if you get stuck!
Thanks @SohamRatnaparkhi - I've passed this over to you. The lighten/darken/rgb were the ones i was on about. Nice spot on the getContrast too! Please give me a shout if you get stuck!
Cool! Will make a PR soon.
Hi @sean-brydon My PR fails because it doesn't pass the tests for branding colors. On checking as to why it doesn't pass, I found out that the library functions return completely different light and dark colors based on the intensity. Results with respect to the test in the brandedColors.test.ts file -> link
With respect to lightMap
:
Light Map check
expected - #fff3f3 got - #fff
expected - #ffe8e8 got - #fff
expected - #ffc5c5 got - #fff
expected - #ffa2a2 got - #fff
expected - #ff5c5c got - #ffafaf
matches - #ff1616
expected - #e61414 got - #000
expected - #bf1111 got - #000
expected - #990d0d got - #000
expected - #7d0b0b got - #1b0000
With respect to dark map:
Dark Map check
expected - #f2f5ff got - #fff
expected - #e6ecff got - #fff
expected - #bfcfff got - #fff
expected - #99b3ff got - #fff
expected - #4d79ff got - #99b3ff
matches - #0040ff
expected - #003ae6 got - #000
expected - #0030bf got - #000
expected - #002699 got - #000
expected - #001f7d got - #000105
To generate these results, I had made a short test script locally. the hex-code following the word expected is the hexcode according to the expectedResultsMap and hex-code following the word got is the code that the library functions return.
Hi @sean-brydon My PR fails because it doesn't pass the tests for branding colors. On checking as to why it doesn't pass, I found out that the library functions return completely different light and dark colors based on the intensity. Results with respect to the test in the brandedColors.test.ts file -> link
With respect to
lightMap
:Light Map check expected - #fff3f3 got - #fff expected - #ffe8e8 got - #fff expected - #ffc5c5 got - #fff expected - #ffa2a2 got - #fff expected - #ff5c5c got - #ffafaf matches - #ff1616 expected - #e61414 got - #000 expected - #bf1111 got - #000 expected - #990d0d got - #000 expected - #7d0b0b got - #1b0000
With respect to dark map:
Dark Map check expected - #f2f5ff got - #fff expected - #e6ecff got - #fff expected - #bfcfff got - #fff expected - #99b3ff got - #fff expected - #4d79ff got - #99b3ff matches - #0040ff expected - #003ae6 got - #000 expected - #0030bf got - #000 expected - #002699 got - #000 expected - #001f7d got - #000105
To generate these results, I had made a short test script locally. the hex-code following the word expected is the hexcode according to the expectedResultsMap and hex-code following the word got is the code that the library functions return.
Interesting - Thanks for giving this a shot. Us at Cal.com are currently on a retreat to improve team working skills.
I will see if i can fix this based on what you have contributed here. Weird how it doesnt work here
Okay cool! Ping me if required.
This is super weird - I can't figure out why we dont get the expected results as they do the same thing
This is super weird - I can't figure out why we dont get the expected results as they do the same thing
I think, they are doing something different from what we do. Link to lighten code
By the way, I am sharing down my testing script below.
const { rgb, lighten, darken , getContrast, getLuminance } = require("polished");
const lightColor = '#ff1616'
const darkColor = '#0040ff'
const createColorMap = (brandColor) => {
const response = {
500: `#${brandColor}`.replace("##", "#"),
};
const intensityMap = {
50: 0.95,
100: 0.9,
200: 0.75,
300: 0.6,
400: 0.3,
600: 0.9,
700: 0.75,
800: 0.6,
900: 0.49,
};
[50, 100, 200, 300, 400].forEach((level) => {
response[level] = lighten(intensityMap[level], brandColor);
});
[600, 700, 800, 900].forEach((level) => {
response[level] = darken(intensityMap[level], brandColor);
});
return response;
};
const lightMap = createColorMap("#ff1616");
const darkMap = createColorMap("#0040ff");
const expectedResult = {
light: {
"50": "#fff3f3",
"100": "#ffe8e8",
"200": "#ffc5c5",
"300": "#ffa2a2",
"400": "#ff5c5c",
"500": "#ff1616",
"600": "#e61414",
"700": "#bf1111",
"800": "#990d0d",
"900": "#7d0b0b",
},
dark: {
"50": "#f2f5ff",
"100": "#e6ecff",
"200": "#bfcfff",
"300": "#99b3ff",
"400": "#4d79ff",
"500": "#0040ff",
"600": "#003ae6",
"700": "#0030bf",
"800": "#002699",
"900": "#001f7d",
},
// ...
};
// check for difference
console.log("Light Map check")
for (const key in expectedResult.light) {
if (expectedResult.light.hasOwnProperty(key)) {
const element = expectedResult.light[key];
console.log(element !== lightMap[key] ? "expected - " + element + " got - " + lightMap[key] : "matches - " + element)
}
}
console.log("\nDark Map check")
for (const key in expectedResult.dark) {
if (expectedResult.dark.hasOwnProperty(key)) {
const element = expectedResult.dark[key];
console.log(element !== darkMap[key] ? "expected - " + element + " got - " + darkMap[key] : "matches - " + element)
}
}
console.log(expectedResult.light === lightMap)
console.log(expectedResult.dark === darkMap)