Skip to content
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

Shadow mode #19

Open
filipbech opened this issue Dec 5, 2017 · 7 comments
Open

Shadow mode #19

filipbech opened this issue Dec 5, 2017 · 7 comments

Comments

@filipbech
Copy link
Contributor

I was thinking that maybe we should make the shadowRoot optional all together?
and maybe make the shadow-mode configurable (open/closed)?

@kenchris
Copy link
Owner

kenchris commented Dec 5, 2017

Sure why not, how would this look?

@filipbech
Copy link
Contributor Author

I was hoping you had an opinion about that :-D Something like withProperties would be the obvious choice but we shouldn't really stick more on that - I don't think so.

Or maybe we should use more of a mixin-pattern? or have two seperate lit-element's that both enherits from a lit-base-element'ish..

do you see other ideas?

@kenchris
Copy link
Owner

kenchris commented Dec 5, 2017

I tried doing mixins.... but doing a JS mixin from TypeScript is apparently impossible... spent quite some time on this and found out that a lot of clever people had spent an equal amount of time and given up :-(

@emolr
Copy link

emolr commented Apr 2, 2018

Hi, I've been playing around with web components quite a lot.
I have been trying an approach where i'll wait with creating shadow until the initial render in invalidate();

So it looks like this:

  async invalidate() {
    if (!this._needsRender) {
      this._needsRender = true;
      this._needsRender = await false;
      
      if (!this._hasValidated && this._shadow) {
        this._root = this.attachShadow({ mode: this._shadowMode });
      }

      this.renderer();
      this._hasValidated = true;
    }
  }

I found this as the only solution if i didn't make my base class into a mixin where i could check if (super.shadow) etc...

I'm not 100% sure on performance impact, but so far it seems good, but i'd like to hear if it's a bad idea, and why if so :)

@kenchris Btw, great work on this project! I've used it as a great inspiration for creating my own library which is similar, but has optional renderers like preact or lit-html :)

@kenchris
Copy link
Owner

kenchris commented Apr 2, 2018

Why not just check if this.shadowRoot exists? You also shouldn't need this._root.

I don't really see a problem with creating the root at this point as it should be a fast operation.

@emolr
Copy link

emolr commented Apr 3, 2018

@kenchris Thanks for the idea of checking if shadowRoot exists, it works flawlessly 👍
And thanks for feedback on the idea :)

@ernsheong
Copy link

ernsheong commented Apr 9, 2018

This is how PolymerLab's lit-element is doing it:
https://github.com/PolymerLabs/lit-element/pull/19/files

To turn off shadow DOM:

// Override _createRoot
_createRoot() {
  return this;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants