-
Notifications
You must be signed in to change notification settings - Fork 26
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
[design-tokens] Design token initial - colors #54
[design-tokens] Design token initial - colors #54
Conversation
"color": { | ||
"background": { | ||
"base": { "value": "{color.base.white.value}" }, | ||
"alt": { "value": "{color.core.neutral.1.value}" }, |
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.
This is the gray background we use some places. Not sure what to call it.
"button": { | ||
"primary": { | ||
"base": { "value": "{color.base.green.value}" }, | ||
"hover": { "value": "{color.base.green.value}" }, |
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.
We have a lot in the css right now to adjust the color:
&:hover {
background-color: $crl-green-4;
color: $crl-base-white;
}
&:active {
background-color: $crl-green-4;
border-width: 1px;
border-color: mix(black, $crl-green-4, 10%);
}
&:focus {
border-width: 1px;
border-color: mix(black, $crl-green-4, 20%);
@include crl-box-shadow(0, 0, 0, 3px, rgba($crl-base-green, 0.4));
}
&:disabled {
background-color: $crl-green-2;
border-color: $crl-green-2;
&:hover {
cursor: not-allowed;
}
}
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 wonder if we could add an additional layer to the states with separate colors. For example for hover
,
...
"hover": {
"background": { "value": "{color.base.green.value}" },
"foreground": { "value": "{color.base.white.value}" },
},
I think should also move ahead and iterate. There may be some way to capture what Rachel had mentioned this morning about a number value representing a percent darker or whathaveyou.
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.
Right now the text color is separate from the background. For example, the button text color on hover is here. Is that what you mean by foreground?
aa18269
to
8b6c8ad
Compare
{ | ||
"color": { | ||
"font": { | ||
"1": { "value": "{color.core.neutral.8.value}" }, |
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.
These are the color variables used for text in figma
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.
LGTM
I had one comment about future iterations, but I think we should start integrating tokens into components as soon as we can.
"button": { | ||
"primary": { | ||
"base": { "value": "{color.base.green.value}" }, | ||
"hover": { "value": "{color.base.green.value}" }, |
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 wonder if we could add an additional layer to the states with separate colors. For example for hover
,
...
"hover": {
"background": { "value": "{color.base.green.value}" },
"foreground": { "value": "{color.base.white.value}" },
},
I think should also move ahead and iterate. There may be some way to capture what Rachel had mentioned this morning about a number value representing a percent darker or whathaveyou.
design-tokens // add design token package
Related to #27
Uses style-dictionary to transform tokens defined in json into SCSS and JS variables.
See #62 for usage.
Example output:
Checklist
I have written or updated test for the changes I madeI have added or updated Storybook if appropriate for my changes