Does this code go against how lit-element should be used?

[mercmobily]

Hi,

I am creating an issue because I feel that if this is a good idea, it should catch the maintainers' attention; and if it's a bad idea, it should be "officially bad".

I want to create some elements (who doesn't?) but at the same time I want to give designers a chance to really theme them (again, who doesn't?). I realise there are custom properties etc., but I don't think they go quite far enough.

This is what I want the end result to be:

        <fancy-input-text label="The label">
          <style slot="style">
            #label {
              color: blue
            }
          </style>
        </fancy-input-text>

I achieved this by writing StyleableMixin.js:

import { html } from 'lit-element'

export const StyleableMixin = (base) => {
  return class Base extends base {
    firstUpdated () {
      super.firstUpdated()

      const styleSlot = this.shadowRoot.querySelector('#style-slot')
      if (styleSlot) {
        for (const styleElement of styleSlot.assignedElements()) {
          if (styleElement.tagName === 'STYLE') {
            this.shadowRoot.appendChild(styleElement)
          }
        }
      }
    }

    get customStyleSlot () {
      return html`
          <slot name="style" id="style-slot"></slot>
        `
    }
  }
}

So, FancyInputText.js that uses the mixin is simply:

import { LitElement, html } from 'lit-element'
import { StyleableMixin } from './mixins/StyleableMixin.js'

export class FancyInputText extends StyleableMixin(LitElement) {
  static get properties () {
    return {
      label: { type: String }
    }
  }

  render () {
    return html`
      ${this.customStyleSlot}
      <label id="label" for="native">
        <div id="label-text">${this.label}</div>
      </label>
      <input type="text" id="native">
    `
  }
}
customElements.define('fancy-input-text', FancyInputText)

Basically, StyleableMixin will copy over to the shadow DOM any style tags defined in in the slots named style. Designers have a way to really change whatever they like about an element, without getting FOUC.

Is this an anti-pattern? Am I doing something absolutely terrible? Or is this what it looks like it is: the holy grain of allowing designers to really change the CSS when custom properties don't go far enough?

Please feel free to throw tomatoes at me if I deserve it!

[mercmobily]

This could be improved further by adding the slot programmatically rather than having to define it in the template.

I frankly haven't attempted that route yet, as I needed a way to theme my own components rather than providing a generic mixin to allow styling.

Also, I would much rather use a function to add an entry to the adoptedStyles pseudo-array; however, this would require a change in lit-element -- the problem being that in lit-element styles() is static function, whereas the extra styles would be a per-element thing.

[lkraav]

Maybe look into what https://jelements.netlify.com/stylable-mixin is thinking about cc @jouni

[mercmobily]

@lkraav I like this one better.
This is an improvement of my original code, where an element will simply have to mixin without worrying about having extra things in the template:

export const StyleableMixin = (base) => {
 return class Base extends base {
   firstUpdated () {
     super.firstUpdated()

     // Add the equivalent of
     // <slot name="style" id="style-slot"></slot>
     // To the shadow DOM
     const styleSlot = document.createElement('slot')
     styleSlot.setAttribute('name', 'style')
     styleSlot.setAttribute('id', 'style-slot')
     this.shadowRoot.appendChild(styleSlot)

     // If an element has one or more <any-tag slot="style"> in its
     // light DOM, the newly added styleSlot will have
     // them  as an assigned element.
     // Clone over all of the ones where any-tag is `style`.
     // So, any <style slot="style"> will be cloned over
     for (const styleElement of styleSlot.assignedElements()) {
       if (styleElement.tagName === 'STYLE') {
         this.shadowRoot.appendChild(styleElement)
       }
     }
   }
 }
}

So ANY mixing element will be styleable like this:

        <fancy-input-text label="The label">
          <style slot="style">
            #label {
              color: blue
            }
          </style>
        </fancy-input-text>

You can even import external CSS:

        <fancy-input-text label="The label">
          <style slot="style">
            @import 'cust/custom-input.css';
          </style>
        </fancy-input-text>

Although you end up with FOUC (but there is no way around it for now, not till CSS modules exist)

[jouni]

To your original point: no, I don’t think this is an anti-pattern. The current ways of styling web components are lacking. The new ::part() pseudo-element helps, but even together with the proposed ::theme() I don’t see how they would solve the issue conveniently.

So, we need more powerful ways for designers to style web components. And for the time being that means injecting styles inside the shadow DOM.

That said, perhaps we shouldn’t open the door to modifying all the internals of a component. The way we do it with Vaadin components is that we clearly define which elements the user is free to style, the styling API of the component. Basically only the [part] and [state] selectors (and some of their extensions like pseudo-classes and pseudo-elements) are considered stable, and you should not use any other selectors when customizing the components. Pretty much what ::part() is about.

The problem I see in your approach is that it requires the designer to write additional HTML for each instance of a component in order to have them styled. Using @import reduces some of the copy paste, but there’s still a big chance of forgetting to add the customization to some instances and then you get a visually inconsistent/broken app.

---

Regarding my own StylableMixin prototype, I have to point out the improvements I’ve been working for it. I have a preview of that available here: https://v0-4--jelements.netlify.com/util/stylable

Basically, it allows you to write stylesheets in all the ways you can: <style>, <link>, @import. So it’s more flexible, not forcing designers to write CSS-in-HTML (let alone CSS-in-HTML-in-JS 🤮).

Modifying @mercmobily’s examples:

<style>
  @media fancy-input-text {
    #label {
      color: blue;
    }
  }
</style>

<link rel="stylesheet" href="cust/custom-input.css" media="fancy-input-text">

Or alternatively:

<style>
  @import "cust/custom-input.css" fancy-input-text;

  @media fancy-input-text {
    #label {
      color: blue;
    }
  }
</style>

You can structure and import your stylesheets pretty much however you want. Traditional style sheets (<link>) should block rendering as well, if I remember correctly, so it could even help with FOUC. Haven’t actually thought about that until now.

One might argue that this is abusing media queries, and they’d be right 😅 But it’s only meant as a temporary workaround. And for the time being it works quite nicely IMHO.

[mercmobily]

@jouni I have to be honest, and I say this in the humblest way possible... I think I prefer my solution. The problem I have with it is exactly what you mentioned: you can't have the same style for everybody without causing FOUC. (using @import will cause FOUC).

I also have to say that ::part and ::theme have basically zero support, and I think there is a chance they might not stick, even in 5 years. I just don't see designers really getting into them -- but I might be wrong.

[jouni]

Sure, all good – I probably sounded a bit defensive in my response, apologies for that (I’ve been thinking about this problem on and off during the 2–3 years or so, so it’s sometimes hard for me to keep a clear vision).

It’s good that we explore different alternative workarounds, as it seems we agree about the current state having problems regarding theming/styling 🤗

And true, I looked a bit closer, and there are nasty timing issues with external style sheets (either <link> or @import) that my prototype does not handle. Also, any light dom that depends on shadow DOM based styling will easily suffer from FOUC before the element upgrades.

::part has support in Chrome. As a designer myself, I see it’s a good tool to have, but does not necessarily solve all my issues. Haven’t used it enough to say for certain. So yeah, let’s see if it actually sticks.

[web-padawan]

I also have to say that ::part and ::theme have basically zero support

I think ::part would be a thing pretty soon (already behind the flag in Firefox, and Safari TP 94).
So hopefully we would be able to start using in next year.

[kevinpschaaf]

Just to sum up and close out this discussion:

• To the extent that the user code you wrote on top of LitElement is working, it's user code and is not particularly abusing LitElement's API's; in short: "it's something one could do"
• Note that dynamically adding <style> to shadowRoots is not currently supported by the ShadyCSS polyfill, so caveat emptor
• Regarding the need your code is expressing, there are a number of other issues you can track for more standard/supporting ways of achieving similar goals:
  • Theming options for shadow roots
  • Finish proposal and POC for LitElement theming API
  • Polyfill CSS Shadow Parts (::part)

[mercmobily]

Thanks for the input Kevin. Just one thing:

Note that dynamically adding <style> to shadowRoots is not currently supported by the ShadyCSS polyfill, so caveat emptor

But shadyCss shouldn't have anything to do with this -- right? I mean, what do you mean by "It's not currently supported by the ShadyCSS polyfill"? What will happen if an older browser using the ShadyCSS polyfill has a <style> added? My impression was that it should simply work, since the browser will pick it up -- am I missing something?

[mercmobily]

@kevinpschaaf Just tagging you in case you missed my previous message. I am trying to look at existing issues about it. One question I have is: is there an API call I can use to "wake up" ShadyCSS to the new styles? Since my code is specifically for lit-element and web components, I can adjust my code to make the right call (if there is one)

[kevinpschaaf]

In short, not currently. ShadyCSS style scoping is designed to work at a class level on the styles in an element class’s template (via the prepareTemplate API, which lit-html’s shady-render abstracts over), which scopes the styles based on the element name. It does not support instance-level style scoping, which would require scoped styles that target individual instances.

The CSS Custom Property Shim does have to do a form of per-instance style targeting, but it is very slow as a result despite a lot of complexity to cache styles between instances, and we’ve held back from adding even more complexity to support instance-time <style> support given the complexity and performance footguns.

[mercmobily]

Hi, Ouch OK. So... wait, adding the style tag has _no_ effect? Doesn't the browser even register it at all? I thought so, since it's clear CSS into HTML! I mean, it might not support CSS properties etc., but doesn't it support straight CSS? Merc.

…

On Thu, 23 Jan 2020 at 11:18, Kevin Schaaf ***@***.***> wrote: In short, not currently. ShadyCSS style scoping is designed to work at a *class* level on the styles in an element class’s template (via the prepareTemplate API), which scopes the styles based on the element name. It does not support instance-level style scoping, which would require scoped styles that target individual instances. The CSS Custom Property Shim does have to do a form of per-instance style targeting, but it is very slow as a result despite a lot of complexity to cache styles between instances, and we’ve held back from adding even more complexity to support instance-time <style> support given the performance footguns. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#845>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQHWXRKACYUR4YOAAHKBW3Q7EEA5ANCNFSM4JII7KZA> .

[mercmobily]

@kevinpschaaf O forgot to tag you again. Please accept my apologies for pestering; I would like to release a project and am trying to make a decision about this one...

[mercmobily]

It all comes down to the dreaded IE11 support...

[kevinpschaaf]

adding the style tag has no effect?

In short, yes, but not the effect you want.

Remember that ShadyCSS is polyfilling the new CSS syntax and semantics for shadow DOM that does not exist in IE11. Consider shadow styles like this for my-element:

<style>
  :host([disabled]) { opacity: 0.5; }
  div { color: red; }
</style>

If you just appended this into the DOM, IE11 has no idea what :host means (would ignore that rule) and would apply the div rule to all divs on the page (remember, in a browser without shadow DOM, to the styling engine every <style> tag in the document applies to the whole page).

As such, when we call ShadyCSS.prepareTemplate to scope this for a specific element class, like my-element, ShadyCSS turns it into something like this (note that this goes along with other code in prepareTemplate that also adds scoping classes -- the .my-element class, to all nodes in a class's template):

<style>
  my-element[disabled] { opacity: 0.5; }
  div.my-element { color: red; }
</style>

However, those styles apply to all <my-element> instances on the page (which is what we want given they are styles defined statically for the element class). However, you're positing slotting specific <style> tags into specific instances, meaning they shouldn't apply to all <my-element> tags, only specific ones. In order to support this, the polyfill would then need to perform instance scoping classes and style transforms, perhaps something like this:

<style>
  my-element[disabled].instance-42 { opacity: 0.5; }
  div.my-element.instance-42 { color: red; }
</style>

Now rather than generating new <style> content once per class, we'd have to do it per instance, and add scoping classes to all nodes in the instance in reaction to adding a <style>, and because all that is way to slow to do just as a matter of course, add a bunch of caching and de-duping logic. Just to reiterate:

The CSS Custom Property Shim does have to do a form of per-instance style targeting, but it is very slow as a result despite a lot of complexity to cache styles between instances, and we’ve held back from adding even more complexity to support instance-time <style> support given the complexity and performance footguns.

[mercmobily]

@kevinpschaaf First of all, I wanted to say THANK YOU for responding and supporting us. It all makes sense now. It's all obvious -- now that I read it. Really, heart-felt thank you.

I can see :host has 86% coverage (https://caniuse.com/#search=%3Ahost). Shadow DOM v1, on the other hand, has 89.89% coverage (https://caniuse.com/#feat=shadowdomv1).

I could provide this <style> option in my mixin (see my original post to see what I want to allow them to do), and basically warn them that old browsers using ShadyCSS polyfill can provide their style, but they will possibly leak out of the "shady DOM" so to speak. (but, this will only affect nodes that are children to the one with the injected style... right?)

...

Or wait, I am wrong, scoped <style> tags are not a thing anymore, so regardless of where they are, <style> will affect the WHOLE document, right?

EDIT: Ah, I now totally get your sentence "(remember, in a browser without shadow DOM, to the styling engine every <style> tag in the document applies to the whole page)."

I guess I could work harder and work out a way to assign a unique class to each element, and allow developers to store the specific ones.