-
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
feat: added $layer token in the background-color #14726
feat: added $layer token in the background-color #14726
Conversation
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Hey @tay1orjones I put this to review, but I'm not sure if the storybook looks good enough. I mean, everything works fine, the thing is that I couldn't rewrite the token. I was trying to rewrite the Looks like we can't rewrite since the module was already loaded, so it can't be configured using "with". Not sure if the helper function you mentioned was something like that. carbon/examples/custom-theme/src/index.scss Lines 6 to 14 in acead26
|
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.
Sorry I missed this @guidari, based on what you're saying I think we're actually okay here. To rewrite this background, you'd need to globally redefine the $layer
token when @use
ing the @carbon/react/scss/theme
the first time.
I think once this is merged we could create a stackblitz example that redefines $layer
, it's sort-of difficult to override the tokens for testing within storybook
packages/styles/scss/components/structured-list/_structured-list.scss
Outdated
Show resolved
Hide resolved
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.
Nice! This looks correct, but the changes to typescript-config-carbon and the d.ts
files shouldn't be in here. If these are from a yarn build
, I'm not sure why they're not ignored and were included here 🤔
…-list-background' of github.com:guidari/carbon into 5410-structured-list-background
0f090f0
…m#14726) * feat: added layer toke in the background color * fix: remove scss * fix: fixed vrt test name * fix: added background style * fix: removed storybook and vrt testing * fix: added layer stories * fix: removed wrong files --------- Co-authored-by: Taylor Jones <tay1orjones@users.noreply.github.com> Co-authored-by: Andrea N. Cardona <cardona.n.andrea@gmail.com>
Closes #5410
Added the $layer token to the background-color, so the user can override the background using tokens.
Changelog
New
Testing / Reviewing
Make sure the change is right. You can inspect the code to see the layer in the background