Clean-up semantic colors using color-mix #1741
Conversation
Not sure if this is really nice |
| /** | ||
| * Utility function to apply transparency to a color (variable) using `color-mix` | ||
| * @param {string} color - The color to make transparent. Can be a theme variabale, a hex code or "currentColor" | ||
| * @param {number} opacity - The desired opacity, a number between 0 and 1 | ||
| */ | ||
| export const transparentize = (color: string, opacity: number) => { | ||
| const parsedColor = color === 'currentColor' ? 'currentColor' : | ||
| color.charAt(0) === '#' ? color : | ||
| color.indexOf('var(--') !== -1 ? color : `var(--${color})` | ||
|
|
||
| const parsedOpacity = 100 - (opacity * 100) | ||
|
|
||
| return `color-mix(in srgb, ${parsedColor}, transparent ${parsedOpacity}%)` | ||
| } | ||
|
|
||
| export const transparentGradient = (color: string, direction: string, fromOpacity: number, toOpacity: number) => { | ||
| const parsedColor = color === 'currentColor' ? 'currentColor' : | ||
| color.charAt(0) === '#' ? color : | ||
| color.indexOf('var(--') !== -1 ? color : `var(--${color})` | ||
|
|
||
| const parsedFromOpacity = 100 - (fromOpacity * 100) | ||
| const parsedToOpacity = 100 - (toOpacity * 100) | ||
|
|
||
| return `linear-gradient( | ||
| ${direction}, | ||
| color-mix(in srgb, ${parsedColor}, transparent ${parsedFromOpacity}%), | ||
| color-mix(in srgb, ${parsedColor}, transparent ${parsedToOpacity}%) | ||
| )` | ||
| } No newline at end of file |
There was a problem hiding this comment.
I would put them in their own files, import them here and then export them from colors/index.ts.
| /** | ||
| * Utility function to apply transparency to a color (variable) using `color-mix` | ||
| * @param {string} color - The color to make transparent. Can be a theme variabale, a hex code or "currentColor" | ||
| * @param {number} opacity - The desired opacity, a number between 0 and 1 | ||
| */ |
There was a problem hiding this comment.
I think this should live inside the commit message instead of a comment.
There was a problem hiding this comment.
The plus side of this for me was that the tooltip of the function will explain what is needed, which I thought would be helpful for the opacity specifically (there are a number of correct ways to pass that through). Any alternative method for that?
There was a problem hiding this comment.
I do like it tbh, but since it's not a practice in my codebase it seems a bit inconsistent to me 🤷 but yeah not sure what the others think.
|
I'm not really sure if we should introduce utility functions like this instead of a set of colors/values defined in the theme 🤔 Could you maybe explain that in your PR description why you would go for this solution? |
| // Action | ||
| --action: var(--earth400); | ||
| --actionBackground: var(--earth50); | ||
| --actionBackground: ${transparentize(color.action, opacity.statusBackgroundColor)}; |
There was a problem hiding this comment.
Should this always have an opacity? It introduces this bug again:
There was a problem hiding this comment.
Yeah, will use a new activeBackground here instead so we can distinguish the two use-cases
Went with a quick solution, we can do anything.
I'm not quite sure what you mean. Are you referring to the removal of |
| background-color: transparent; | ||
| border-radius: calc(${radius.md} + 1px); | ||
| box-shadow: 0 0 0 ${space[4]} ${color.actionFocus}; | ||
| outline: ${space[4]} solid ${transparentize(color.action, opacity.statusFocusColor)}; |
There was a problem hiding this comment.
The reason for this change is because color-mix doesn't play nicely with css variables + box-shadow (yet)
@Marruk this indeed! |
Ah, that was just an idea to keep the code a bit cleaner since the syntax of |
2 participants
Recently color-mix got supported by all major browsers.
It has the potential to clean up our semantic colour set by basing variants on a smaller set of base colours. Some more context here.
This PR is a POC taking the
actionBackgroundandactionFocusas examples.I added two utility functions to keep the code cleaner as the syntax of
color-mixis a bit long, but those are optional of course.