-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(components): update get-token-value helper to error out at end of loop #5838
Conversation
Deploy preview for carbon-elements ready! Built with commit bb11595 |
Deploy preview for carbon-components-react ready! Built with commit bb11595 https://deploy-preview-5838--carbon-components-react.netlify.com |
Quick question @laurenmrice, for tags what should we do if someone has written a custom theme? Would there be a good set of fallback colors we could use? |
Deploy preview for carbon-elements ready! Built with commit fefc853 |
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.
Working as expected 👍 ✅
Deploy preview for carbon-components-react ready! Built with commit fefc853 https://deploy-preview-5838--carbon-components-react.netlify.com |
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 👍 - Thanks @joshblack!
@joshblack What can we do with tags for theming? I always just assumed they would have to override the colors. Creating tokens for each tag color seems excessive. |
Chatted with @aagonzales real quick on Slack and she said that fallback colors should come from the white theme 👍 We might need to do two additional things for custom theme support:
|
… loop (carbon-design-system#5838) * Update _component-tokens.scss * Update _component-tokens.scss * fix(component-tokens): update helper and add test suite for custom themes * fix(components): update emit-component-tokens and move from themes Co-authored-by: TJ Egan <[email protected]>
… loop (#5838) * Update _component-tokens.scss * Update _component-tokens.scss * fix(component-tokens): update helper and add test suite for custom themes * fix(components): update emit-component-tokens and move from themes Co-authored-by: TJ Egan <[email protected]>
This fixes an issue consumers were having with 10.11.0 where
get-token-value
was error'ing out. It seems like the@error
block was included in the@each
block which causes it to throw if the theme was not the first match. This PR moves the@error
call out of this loop such that if nothing is returned by the loop it throws. This also cleans up the error messages to be more detailed.Changelog
New
Changed
_component-tokens.scss
to account for matching on the theme across all iterationsRemoved
Testing / Reviewing
$carbon--theme--white
and verify that it runs