Shoelace: Toast stacks appear outside of sl-theme

Created on 16 Jan 2021  Â·  14Comments  Â·  Source: shoelace-style/shoelace

I've wrapped everything in <body> within an <sl-theme name="dark"> tag. However, when the toast stack is used for alerts, because that toast stack itself is appended to body directly, it doesn't pick up that theme.

A simple workaround is to add class="sl-theme-dark" to <body> directly and not use the theme tag, but it'd be nice to have the stack detect a top-most theme tag and append to that instead…

bug

Most helpful comment

I've appreciated the convo here…for the record, I'm firmly in the don't want CSS-in-JS camp. It's cool you can use those techniques if and where required, but I certainly wouldn't want it to be a necessary approach for the library as a whole.

All 14 comments

There are some known issues with <sl-theme> that need to be rethought, and I'm not sure what the ideal solution is yet. The component currently just wraps the sl-theme-name class around so using the class directly on body is the best non-experimental way right now.

The idea behind the theme component was that you'd be able to use multiple themes on the same page. While this is somewhat true in its current state, the caveat is that it requires an element within the body and there's no way to determine which theme is the primary theme (e.g. which theme things like the toast stack prefer).

I think for now the docs should be reverted to show the old method, which is <body class="sl-theme-name"> — at least until the theme component can be fixed to work the way it's intended.

The other problem with <sl-theme> is that you can't opt back in to the light theme because there _isn't_ a light theme — just the default theme which is really just some initial values to make the components look nicer.

<sl-theme name="dark">
  ...
  <sl-theme name="light">Nope!</sl-theme-name>
</sl-theme>

Of course, folks don't take the time to restyle everything. Should they really incur this burden? Probably not. But either way there tends to be an expectation that the above will work.

It might make sense to scrap <sl-theme> altogether and expose a utility that lets you toggle themes by updating the body class. Maybe multiple themes on the same page is something we just can't support _today_. Does anyone even want that? IIRC it's something I just thought would be cool, but I'm not aware of any libraries that do this nor users that were asking for it.

Maybe sl-theme could be useful for defining mini-themes in different pages/sections, but as a mechanism for site-wide theming I'm not so sure. Agree with all your points.

Is the current way of theming in SL even capable of theming third-party components (for example a date picker) properly out of the box? Means, if you include ".../dark.css" will that date picker magically be shown in dark theme as well or do you have to customize all those date picker parts by hand (=> .sl-theme-dark third-party-date-picker::part(base) { ... })?
Maybe I am wrong, but I doubt that this will ever work out of the box in the current way.
Wouldn't it be much better not to use any external CSS theme files but to have a new exported type Theme in SL that holds all the token values for the current themes simular to https://baseweb.design/guides/theming/#createtheme or https://fabricweb.z5.web.core.windows.net/pr-deploy-site/refs/heads/master/theming-designer/index.html (you have to click on "Export theme" in the top-right corner)? And alsoexport const darkTheme = ... and export const lightTheme = ... would be very helpful and also a core createTheme function.
Then sl-theme should be rewritten to be used like <sl-theme theme={darkTheme}>.
At least, this would help to fix all the above mentioned issues.

BTW: I think a light-themed app with dark-themed left sidebar is a common use case for theme switching inside of one app.

I originally wanted users to be able to scope themes to containers, allowing multiple themes on the same page, but this introduces other problems as we're seeing here. I'm not sure people even want this, so I'm OK with backpedaling on that idea.

Let me take a step back to explain what the "default theme" actually is. First off, it's not a theme, it's a collection of root-level design tokens that _are not_ connected to specific components. Instead, components utilize design tokens to provide consistent base styles throughout the library.

It's worth noting that Shoelace isn't intended to be used with its default styles. It's _expected_ and _encouraged_ that users customize it, but at the same time it doesn't make sense to require them to style every component from scratch.

As a result, there are two ways you can customize the library. I've explained this in detail on this page. In short, design tokens offer a high-level way to restyle things that affect _all_ components and components parts offer a low-level way to target individual components.

Is the current way of theming in SL even capable of theming third-party components (for example a date picker) properly out of the box?

Yes, for high-level theming. Components just need to use the design tokens properly. For lower-level theming, it's fair to assume that some tweaking will usually be necessary.

The dark theme, for example, has a lot of ::part() selectors because it's such a drastic change from the default theme. You could, however, achieve a similar result by inverting color tokens. This has been demonstrated and I responded with my concerns.

I don't really like the idea of inverting color tokens because their lightness values no longer make sense, but perhaps that's worth exploring a bit more.

Wouldn't it be much better not to use any external CSS theme files but to have a new exported type Theme in SL that holds all the token values for the current themes

I don't see how that helps, and I'd prefer to stick with basic CSS for themes. Complicating it makes it harder to explain, understand, and use.

But maybe I'm not seeing the same vision as you. If you have a simple demo that makes it clearer, I'd be happy to tinker with it. 😄

BTW: I think a light-themed app with dark-themed left sidebar is a common use case for theme switching inside of one app.

Great use case here.

Frankly, I have a very strong opinion about theming and how to weight pros and cons there.
I hope, it's fine for you if I speek openly, because frankly, I disagree in most of what you've said and written regarding theming in Shoelace (I know, I am an annoying person - hope you do not mind too much :wink:).

The main problem, IMHO, is that the Shoelace project mixes up theming and customization:

"Theming" should only mean "a different set of (well-defined) token values (for colors, fonts, etc) " (this has NOTHING to do with part:: selectors).
If a Shoelace "dark theme" cannot be achieved just by setting different token values, then the problem is that the SL theme tokens are not flexible enough and have to be enhanced with some further tokens or whatever will be necessary to fix that issue.

"Customization" on the other hand should be considered everything that has to handle with part:: selectors. While "theming" an app will be quite common (e.g. using the corporate design colors of your company or whatever), "customization" will be much rarer (maybe some SL users will even never need customizations in there apps).

I really don't think I have to implement my proposal above, to show my point, do I?
Isn't that example basically self-explaining? Isn't that a much, much simpler approach?

<sl-theme theme={blueTheme}>
  <sl-button>Blue button</sl-button>
  <sl-theme theme={redTheme}>
    <sl-button>Red button</sl-button>
    <sl-theme theme={greenTheme}>
      <sl-button>Green button</sl-button>
    </sl-theme>   
    <sl-button>Red button</sl-button>
  </sl-theme>
  <sl-button>Blue button</sl-button>
</sl-theme>

PS: Maybe the following could be reconsiderd (I personally think that it is only complicating things and is not really that helpful):

[....] high-level way [...] low-level way [...]

[...] what the "default theme" actually is. First off, it's not a theme [...]

One thing makes me uncomfortable with this approach is how color token values are "inverted"

I don't really like the idea of inverting color tokens because their lightness values no longer make sense

A theme is nothing more than a stylesheet that uses the Shoelace API to customize design tokens and/or components.

I'd prefer to stick with basic CSS for themes.

To create a theme, you will need a decent understanding of CSS, including CSS Custom Properties and the ::part selector

I hope, it's fine for you if I speek openly

I would expect nothing less, and I encourage opposing ideas when they make sense for the project. 😄

The main problem, IMHO, is that the Shoelace project mixes up theming and customization:

I don't think Shoelace mixes them up at all. They're two separate concepts, which is why they're separated into design tokens (theming) and parts (customization).

If a Shoelace "dark theme" cannot be achieved just by setting different token values, then the problem is that the SL theme tokens are not flexible enough and have to be enhanced with some further tokens or whatever will be necessary to fix that issue.

It can be though...and perhaps it should. This is what I'd like to explore more. I'm just not sure if it make sense to have --sl-color-primary-100 be a very light color in a light theme and a very dark color in a dark theme. Does that matter? I'm not sure. Regardless, it's possible to do, I just prefer not to because I'm not sure what the answer to that question is yet.

Furthermore, it's possible there are design tokens that are missing. We should identify those and add them where it makes sense. At the same time, we need to remain cautious because adding too many turns into variable hell and brings us down a path that leads to Bootstrap syndrome.

the problem is that the SL theme tokens are not flexible enough

Regarding this, I'm reluctant to abstract every single use of color in a component's CSS into a design token. There are just too many for that to make sense, but I'll keep an open mind and review the ones that are currently being used.

Regarding "Theming" and "Customization":

I don't think Shoelace mixes them up at all.

Maybe there's just a misunderstanding, but the fact that the official "dark theme" CSS file uses both "CSS custom properties/design tokens" plus "::part" selectors shows exactly what I've meant, that there is currently indeed no real seperation between "Theming" and "Customization" in the Shoelace project (otherwise "::part" selectors would not be necessary for the dark theme):
https://cdn.jsdelivr.net/npm/@shoelace-style/shoelace@2.0.0-beta.26/themes/dark.css

Please find here a very naive, quick'n'dirty implementation of that Theme, createTheme, <sl-theme theme={orangeTheme}> stuff I was talking about above.

https://codesandbox.io/s/cool-sinoussi-bi10e?file=/src/index.ts

This is a complete different (and IMHO much better) way to handle themes. It has some really major advantages:

  • Theming can be used completely dynamically and also hierarchically.
  • There's no need to import externals CSS files (just wrap a <sl-theme ...> tag around your app).
  • There is indeed a "default theme" now: Just use <sl-theme>...</sl-theme> without any theme prop and the default theme will be used.
  • The current design token values are available at run time. This can be useful in some cases (I guess that toast stack issue above may be one of them)
  • Third party components will be shown automatically in the correct theme (as long as there's a <sl-theme ...> around the whole stuff (there's no way that this will work in general with the current "dark theme" without - as you've called it - "some tweaking will usually be necessary")
  • We would be a big step further in making things run out of the box without the need of configuring or including annoying stuff.

Main disadvantages:

  • I guess it will be some work to make the dark theme run properly (but this is not a conceptional problem).
  • The JS size of the SL core will be increased (as the default theme is now part of SL JS core code), but I guess this is not a big deal.
  • Obviously a breaking change.
  • Documentation and tests have to be adapted.
  • I guess there will be some minor changes necessary on the design tokens (at least --sl-color-white and --sl-color-black should be renamed)

PS: Finding a "working out-of-the-box" solution for that "light DOM styles" issue will be necessary but I'm sure this will not be a bigger problem.

I've hidden my latest comment as it might be outdated. Here's a less invasive proposal:

If the sl-theme element had a part, for example part="base", then theming would be much easier.
Please find here a demo with dynamic and hierarchical themig where no CSS file is imported (this "no CSS file necessary" thing is not really important, it's just the way I like it) .
Please be aware that createTheme, loadTheme, lightTheme, orangeTheme and greenTheme are userland only. The only change in SL core would be to add that part to <sl-theme...>.

[Edit: sorry, wrong demo - here's the correct one]
https://jsfiddle.net/o4Ler90a/2/

Thanks for posting the example. I played with it a bit and you're not going to like my response. 🙈

The fiddle demonstrates a way to create and merge themes using JavaScript objects, but I can't see an advantage over CSS and I prefer the cascade over JS/JSON. It's also much easier for users to do all this through CSS, especially when the library is consumed via CDN. I want to stick to the platform, and this concept deviates from that with no obvious benefit.

In fact, the only potential advantage it seems to have is automatically injecting the styles into the <head> instead of letting the user do it, but what if the user wants to minify or optimize their stylesheet somehow? It's totally possible a consumer of only one or two components will strip down the tokens and merge them with a custom theme, then optimize the result before sending it over the wire. This approach takes that control away from them (or at least means they have to start doing it in JS). It also means stylesheets won't be cached, so we'll have to wait even longer before styles hit the DOM.

From what I can tell, it also doesn't let us create a dark mode from design tokens unless you invert the lightness values in colors tokens. We can technically do the same thing with current approach. Here's a fiddle demonstrating this. It loads the default theme and then inverts all gray + primary tokens:

https://jsfiddle.net/claviska/95qLfeas/

You'll notice that --sl-color-white and --sl-color-black are also inverted, which is a great example of why inversion isn't a good method to use. Aside from that, it's common for dark mode themes to not be a direct inverse of light mode. For example, in light mode you might use --sl-primary-500 for a hover color, but in dark mode you'd want it to switch to --sl-primary-600 so it's not as bright and blinding.

Design tweaks just can't be automated like this and I don't believe color tokens should work that way, which is why I authored the dark theme the way I did.

Circling back to the main issue, extracting the toast controller into a utility would be another solution.

import { toastStack } from '@shoelace-style/shoelace';

toastStack.theme = 'dark'; // sets sl-theme-dark class on the toast stack

I've been thinking about extracting toast into a controller anyway, so it could even be merged with that.

import { toast } from '@shoelace-style/shoelace';

const alert = document.querySelector('sl-alert');

toast.show(alert, { theme: 'dark' });

Some things don't make as much sense to be declarative, like toasts and icon libraries. I've been thinking that when an imperative config/method is required, it should probably be extracted into a utility.

Any thoughts on that?


As an aside, something I noticed in your example is that the base part isn't necessary. Since styles are injected into the <head>, both the styles and slotted content <slx-theme> is in the light DOM. Changing these lines produces the same result:

connectedCallback() {
  this.shadowRoot.innerHTML = `
    <slot></slot>
  `
}

// ...

const lines = [`slx-theme[name=${name}] {`];

I played with it a bit and you're not going to like my response.

Actually, I really love your respone :smile:, because of this comment:

I noticed in your example is that the base part isn't necessary.

I've tried something similar in an earlier version and couldn't make it run, so I mistakenly assumed that this is only working with parts. Thank you very much Cory, that really helped me a lot.
Just in case someone's interested, here's the enhanced version of the demo: https://jsfiddle.net/f4oxvgLn/

I do not think it makes any sense to argue about what are the advantages or disadvantages of CSS-in-JS. Once you fall in love with CSS-in-JS / Icons-in-JS etc. then it's really no fun to go back to writing apps that depend on external assets (CSS, icons etc), I personally hate this like hell, feels like a time travel back to the 90ies :wink:. But it's also perfectly fine to hate this "CSS-in-JS/Icons-in-JS" approach. I guess, by the end of the day, it's a matter of taste.
But the important outcome of our discussion is that you have proven that even a CSS-in-JS approach is already possible with Shoelace, although you do not recommend it.

Regarding that question how to implement a dark theme, it seems that we are just of completely opposite opinions (and I still believe it will be no fun to use the dark theme in combination with third-party components). But I guess, like often in life, it's fine to agee to disagree :wink:

I've appreciated the convo here…for the record, I'm firmly in the don't want CSS-in-JS camp. It's cool you can use those techniques if and where required, but I certainly wouldn't want it to be a necessary approach for the library as a whole.

Perhaps it would have been less ambiguous, if I had called it "Dynamic theming in JS/Icons-in-JS" instead of talking about "CSS-in-JS/Icons-in-JS" ... but I guess, like Jared mentioned above, it's a good thing that dynamic theming is possible where required ... or preferred ... anyway, thanks again for the (at least for me :wink:) very helpful discussion ...

For now, I've removed the experimental <sl-theme> component and reverted back to using the sl-theme-[name] class. This was a good discussion though, and I have new ideas moving forward!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

claviska picture claviska  Â·  5Comments

crutchcorn picture crutchcorn  Â·  7Comments

lamplightdev picture lamplightdev  Â·  7Comments

StommePoes picture StommePoes  Â·  3Comments

Xyz3R picture Xyz3R  Â·  7Comments