-
Couldn't load subscription status.
- Fork 0
Clean-up semantic colors using color-mix #1741
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { css } from '@emotion/react' | ||
| import { space, radius, color } from './theme' | ||
| import { space, radius, color, opacity } from './theme' | ||
| import { baseTextStyles } from './components/Text' | ||
| import { transparentize } from './utils/colors' | ||
|
|
||
| export const globalStyles = css` | ||
| :root { | ||
|
|
@@ -73,10 +74,8 @@ export const globalStyles = css` | |
|
|
||
| // Action | ||
| --action: var(--earth400); | ||
| --actionBackground: var(--earth50); | ||
| --actionBackground: ${transparentize(color.action, opacity.statusBackgroundColor)}; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, will use a new |
||
| --onAction: var(--nova); | ||
| --actionFocus: #00b6f052; | ||
| --actionBackgroundAlpha12: #00b6f01f; | ||
|
|
||
| // Success | ||
| --success: var(--titan400); | ||
|
|
@@ -100,7 +99,6 @@ export const globalStyles = css` | |
| --info: var(--earth400); | ||
| --infoBackground: var(--earth50); | ||
| --onInfo: var(--nova); | ||
| --actionFocus: #00b6f052; | ||
|
|
||
| // Inactive | ||
| --inactive: var(--stardust500); | ||
|
|
@@ -292,7 +290,7 @@ export const globalStyles = css` | |
|
|
||
| &:focus { | ||
| outline: 0; | ||
| box-shadow: 0 0 0 ${space[4]} ${color.actionFocus}; | ||
| box-shadow: 0 0 0 ${space[4]} ${transparentize('currentColor', opacity.statusFocusColor)}; | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| /** | ||
| * 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 | ||
| */ | ||
|
Comment on lines
+1
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should live inside the commit message instead of a comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| 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}%) | ||
| )` | ||
| } | ||
|
Comment on lines
+1
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would put them in their own files, import them here and then export them from |
||

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.
The reason for this change is because
color-mixdoesn't play nicely with css variables +box-shadow(yet)