Carbon: Customized theme tokens getting reset by inline-notifications mixin

Created on 29 Jul 2019  Ā·  9Comments  Ā·  Source: carbon-design-system/carbon

What package(s) are you using?

  • [x] carbon-components
  • [ ] carbon-components-react

Detailed description

Describe in detail the issue you're having.

Theme tokens that I have specified a custom color for are being reset by the inline-notifications mixin. For example, say I set the base theme to Gray 10 and set $focus to red (#ff0000). When the inline-notifications mixin sets the __action-button class it uses the carbon--theme mixin to borrow $hover-secondary from the Gray 100 theme. Upon doing so, the current theme gets reset to the base Gray 10 theme, overriding the custom value for $focus in the process. Every subsequent component import in styles.scss then uses the base theme's $focus rather than my custom value.

Is this issue related to a specific component?

Related to inline-notification component (__action-button class).

What did you expect to happen? What happened instead? What would you like to
see changed?

I expected all of the components to use the $focus value I defined. Instead, my $focus value was used in all the components leading up to inline-notification and then was reset. I would like the per-token customizations to persist.

What browser are you working in?

Chrome, although the issue is happening during the scss compilation.

What version of the Carbon Design System are you using?

10.4.1, and @carbon/themes 10.4.0

Steps to reproduce the issue

  1. In the main scss file, import @carbon/themes/scss/theme (bug also present with themes package in vendors folder of carbon-components)
  2. Set $carbon--theme to $carbon--theme--g10
  3. Call carbon--theme mixin
  4. Import carbon-components/scss/globals/scss/styles
  5. Compile scss (I used debug statements to check the value of $focus at different stages)
bug šŸ›

Most helpful comment

Values for $inverse-hover-ui token:

| White | Gray 10 | Gray 90 | Gray 100 |
| --- | --- | --- | --- |
| Gray80 hover #4C4C4C | Gray80 hover #4C4C4C | Gray 10 hover #e5e5e5| Gray 10 hover #e5e5e5 |

I've created an $inverse-hover-ui token with what I think are working values. It needs a bit more thinking so we are not going to publish to the site right now. FYI @aagonzales Ideally we can fix inline theming to work correctly so we don't have to create so many tokens with repeated values. We will see what @mattrosno thinks when he gets to give this a look.

All 9 comments

cc @mattrosno @jendowns

Just tested and also seeing this behavior in the demo environment. Seems to be related to https://github.com/carbon-design-system/carbon/commit/3063cc8e6e6371a15edf404b83c19e40e927e8f1

Any way we can properly scope these theme calls? Seems to be setting the entire theme to the defaults after the inline notifications are called

@tw15egan I wonder if it would help to come up with a token for inverted hover secondary color? That way there’s no need for the theme calls.

(Separately, it would be cool if these theme calls didn’t reset everything like this — I thought the intention is that they are meant to be scoped as-is?)

šŸ‘ on theme token solution rather than theme calls. @shixiedesign Do you think we need to add theme tokens for this problem space? (@jendowns Any refresher for what your past change is for?) Or can existing tokens solve this? Thanks!

@asudoh the change was to get around the lack of a theme token, so a new token could definitely solve this. (Though likewise I think the documentation for theming should be updated, since these mixins are presented as an ā€œinline themingā€ solution here: https://github.com/carbon-design-system/carbon/blob/master/packages/themes/README.md#sass)

I see, thanks @jendowns - What problem did the lack of the theme token cause? This information would help our designers define a new theme token if needed.

@asudoh the background color for the notification is inverted, but there isn’t a similar inverted color for for the hover background color of the button.

I agree I was under the assumption that those theme calls were scoped and could be used as "inline theming". Adding a temporary theme variable for this situation will fix this, but it seems like there is a bug with these theme calls that need to be addressed as well.

@tw15egan @asudoh I proposed a quick fix here, in the hopes that it could be included in the upcoming release if you are okay with it? šŸ˜…

https://github.com/carbon-design-system/carbon/pull/3596

This would remove the inline theme mixins used in the InlineNotification component completely (which resolves this issue) and then use $ui-05 as the hover background color for the action button instead, which seems like an acceptable bandaid? The token isn't explicitly an inverted one, but it's colors are close: https://carbon-elements.netlify.com/themes/examples/preview/

Let me know what you think!

Values for $inverse-hover-ui token:

| White | Gray 10 | Gray 90 | Gray 100 |
| --- | --- | --- | --- |
| Gray80 hover #4C4C4C | Gray80 hover #4C4C4C | Gray 10 hover #e5e5e5| Gray 10 hover #e5e5e5 |

I've created an $inverse-hover-ui token with what I think are working values. It needs a bit more thinking so we are not going to publish to the site right now. FYI @aagonzales Ideally we can fix inline theming to work correctly so we don't have to create so many tokens with repeated values. We will see what @mattrosno thinks when he gets to give this a look.

Was this page helpful?
0 / 5 - 0 ratings