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

[docs] Review CSS Styles page for technical accuracy #353

Closed
2 tasks
ghost opened this issue Dec 11, 2018 · 11 comments · Fixed by #698
Closed
2 tasks

[docs] Review CSS Styles page for technical accuracy #353

ghost opened this issue Dec 11, 2018 · 11 comments · Fixed by #698
Milestone

Comments

@ghost
Copy link

ghost commented Dec 11, 2018

Review the content at https://lit-element.polymer-project.org/guide/styles for technical accuracy only.

Markdown source: https://github.com/Polymer/lit-element/blob/master/docs/_guide/styles.md

  • fix :host vs custom element tag
  • fix/remove info on JS expressions in <style></style>
@ghost ghost added the Area: docs label Dec 11, 2018
@frankiefu
Copy link
Contributor

https://lit-element.polymer-project.org/guide/styles#use-the-custom-element-tag-as-a-selector
The :host CSS pseudo-class has higher specificity than the element’s type selector is incorrect. I think it should be the other way around. See this jsbin

@keanulee
Copy link
Contributor

"Use JavaScript expressions in LitElement style blocks"

<style>${this.myStyles}</style> is considered an anti-pattern since the browser has to re-parse the entire style tag if the textContent changes. Suggested pattern would be to make this.myStyles a complete style tag instead if you really want to call out styling with JS expression (which IMO could be skipped).

@ghost ghost self-assigned this Dec 13, 2018
@johnthad
Copy link

johnthad commented Dec 13, 2018

@keanulee What if style is a constant? As in

connectedCallback() {
  super.connectedCallback();
  const style = `font-family: ${label.font.family};...
  this.checkbox = html`
    <span aria-checked="${this.ariaChecked}">
      <label style="${style}">...
}

render() {
  this.setAttribute('role', 'checkbox');
  return html`
    ${HostStyle} ${this.checkbox}
  `;
}

If style never changes, it shouldn't have to rerender. Yes?

@keanulee
Copy link
Contributor

keanulee commented Dec 13, 2018

Looking at your snippet, there's a limitation on what actually works with JS expressions in style which might make the above code not behave as expected.

  • When using LitElement with Shady DOM (the shim for when there's no native Shadow DOM), the <style> tags returned from render() are only processed once per element class (not instance). So you can't have multiple instances of the same element with different text content of <style>. Use custom properties to style instances instead (e.g. --element-color: red).

Using constants for theme styling across elements is fine, but it's still recommended to make the expression a complete <style> tag. That way, the shared style tag will only be parsed once, even if multiple element classes it. For example:

// 2 style tags with different text content, `partialStyle` parsed twice.
// element-one
html`<style>
  :host {
    color: blue;
  }
  ${partialStyle}
</style>`;

// element-two
html`<style>
  :host {
    color: red;
  }
  ${partialStyle}
</style>`;
// `completeStyle` has same text content, only parsed once.
// element-one
html`<style>
  :host {
    color: blue;
  }
</style>
${completeStyle}`;

// element-two
html`<style>
  :host {
    color: red;
  }
</style>
${completeStyle}`;

@ghost
Copy link
Author

ghost commented Dec 13, 2018

@keanulee Could you clarify in the completeStyle example what's inside that expression? Not sure I understand what's intended

@keanulee
Copy link
Contributor

keanulee commented Dec 13, 2018

completeStyle is partialStyle wrapped with a <style> tag. For example:

const partialStyle = `
:host {
  background: white;
  border: 1px solid black;
}
...
`;

const completeStyle = html`
<style>
  :host {
    background: white;
    border: 1px solid black;
  }
  ...
</style>
`;

Looking back, my example styles are missing selectors. Will edit.

@johnthad
Copy link

Regarding "...the <style> tags returned from render() are only processed once per element class (not instance)": Does that include style attributes as in my example above? If so, this is a difficult limitation of LitElement for the particular problem I'm attempting to solve at the moment. I might have resort to imperative JS to set much of style in a portion of my project.

@keanulee
Copy link
Contributor

style attributes are fine since they only apply to that element and don't do anything special in Shadow DOM (so Shady DOM shim doesn't touch them).

@johnthad
Copy link

Hot damn and hallelujah! You had me worried there. For the particular problem I'm working on--parsing a 3rd party file format and rendering the native form into a browser--there's no limit of the different styles similar elements can have. In Polymer 2, I could put the values directly into my <template>:

<div id="line" style="border-width: [[borderWidth]]px; border-style: [[borderStyle]]; 

I learned early on this was not an option with LitElement. In this portion of my app, where styles are abitrary and out of my control, I've been using style attributes as I build instances on-the-fly.

@djlauk
Copy link
Contributor

djlauk commented Jan 18, 2019

The documentation for styles does not differentiate between the :host and :host() pseudo classes.
As CSS selectors are only valid on :host(), the examples in the LitElement docs (and also on StackBlitz) on respecting the hidden attribute don't work.

Hmm... Maybe (my) words are not good to describe this. Here is, what I mean in a code snippet:

<style>
/* (1) This works. */
:host { border:1px solid red; }

/* (2) This does **not** work. */
:host() { border:1px solid red; }

/* (3) This does **not** work. */
:host[hidden] { display:none; }

/* (4) This works. */
:host([hidden]) { display:none; }
</style>

References:

@dbatiste
Copy link

dbatiste commented Mar 7, 2019

Possible improvement for the docs that I am wondering about:

What is the best practice for handling use of global custom properties in lit-element components, such that they work in IE11? One approach is to simply define them at the document level, and rely on css property fallbacks inside the component for IE11. This is what I'd normally expect (aside form IE11), but it requires components to have the respective values as fallback. An alternative approach would be to define those styles in a css template and import them into the component so that they work IE11 without the component needed to rely on fallback values.

@ghost ghost removed their assignment Mar 18, 2019
@ghost ghost self-assigned this Mar 25, 2019
@ghost ghost added the Status: In Progress label Mar 25, 2019
@dorivaught dorivaught added this to the 2.1 milestone Mar 26, 2019
@ghost ghost mentioned this issue Jun 4, 2019
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.

6 participants