-
Notifications
You must be signed in to change notification settings - Fork 2
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
style: update tabs tokens to pine #2060
Conversation
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.
Non-blocking comments. Thanks for getting this updated!
padding: rem(6px) rem(14px); | ||
background-color: sage-color(grey, 200); | ||
border-radius: sage-border(radius-x-large); | ||
padding: calc(var(--pine-dimension-2xs) * 1.5) calc(var(--pine-dimension-xs) * 1.75); // 4px * 1.5 = 6px, 8px * 1.75 = 14px |
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 posed a question about this in Slack, but I am wondering if we'd like to continue using calcs this way. Would love to know your thoughts on it.
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.
Thoughts in slack as well. For whatever reason there are values here that don't easily convert to dimensions. We can either hard code or calc so they scale or possibly extend dimensions a bit for simpler calculations. 🤷
background-color: sage-color(grey, 200); | ||
border-radius: sage-border(radius-x-large); | ||
padding: calc(var(--pine-dimension-2xs) * 1.5) calc(var(--pine-dimension-xs) * 1.75); // 4px * 1.5 = 6px, 8px * 1.75 = 14px | ||
background-color: var(--pine-color-border-disabled); |
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.
Can we match this background color with Pine?
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.
@anechol Great catch, but I think it might be off in Pine as well. I am seeing #fff in Pine, but #F0F0F0 in Figma. Will change to figma 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.
Just asked design to review this color in Figma so we'll see how they feel.
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.
Sounds good. TBH, it looks like a link with a white background. Ok in Figma with the off-white background of the canvas. Just let me know.
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.
@anechol design has responded. no rush, but let me know how we should proceed.
Description
Converts sage tokens to pine tokens
Testing in
sage-lib
Navigate to tab and tabs
Verify token updates
Testing in
kajabi-products
Related
https://kajabi.atlassian.net/browse/DSS-1269