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

styles are not read from the constructor chain #467

Closed
6 tasks done
tlouisse opened this issue Jan 22, 2019 · 4 comments
Closed
6 tasks done

styles are not read from the constructor chain #467

tlouisse opened this issue Jan 22, 2019 · 4 comments
Assignees

Comments

@tlouisse
Copy link
Contributor

tlouisse commented Jan 22, 2019

Description

Live Demo

https://stackblitz.com/edit/lit-element-request-update-bug-lspgdq?file=test.js

Steps to Reproduce

  1. Create function MyStyleMixin with static get styles defined. For instance:
const MyStyleMixin = (superclass => class MyStyleMixin extends superclass {
  static get styles() { return [
    css`
      div {
        border: 2px solid blue;
      }`
    ];
  }
});
  1. Create class MyStyledElement that applies MyStyleMixin and extends LitElement.
    Define a render function that are styled by MyStyleMixin. Define it as custom element my-styled-element.
    For instance :
class MyStyledElement extends MyStyleMixin(LitElement) {
  render() {
    return html`<div>Styles should be applied here</div>`;
  }  
}
customElements.define('my-styled-element', MyStyledElement);
  1. Instantiate <my-styled-element></my-styled-element> on a page.

Expected Results

<my-styled-element> has a div with a blue 2px border.

Actual Results

<my-styled-element> has no styling at all.

Browsers Affected

  • Chrome
  • Firefox
  • Edge
  • Safari 11
  • Safari 10
  • IE 11

Versions

  • lit-element: v2.0.0-rc.3
  • webcomponents: v2.2.3
@web-padawan
Copy link
Contributor

I have also noticed that while working on web-padawan/lit-components#30. Had to tweak the code when updating to 2.0.0-rc.3 and make sure the class extension in the tests does the following:

static get styles() {
  return super.styles;
}

For me it was an acceptable tradeoff, but in general it looks like a regression.

@tlouisse
Copy link
Contributor Author

@web-padawan : Yeah, I also already did this in my code as a workaround :)
But we have quite a few mixins that share styles and it feels a bit hacky to me if we have to trigger the LitElement like this everytime.

@dfreedm dfreedm self-assigned this Jan 22, 2019
@dfreedm
Copy link
Member

dfreedm commented Jan 22, 2019

The intent is to inherit styles if they have not been set on the subclass:
https://github.com/Polymer/lit-element/blob/master/src/lit-element.ts#L75

@tlouisse
Copy link
Contributor Author

Thanks for fixing the issue @azakus, pretty smart to check every level via finalize 👍 Was there a particular reason to rename it from _finalizeto finalizeif I may ask?

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

Successfully merging a pull request may close this issue.

3 participants