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

Adds static get styles() #401

Merged
merged 15 commits into from
Jan 10, 2019
Next Next commit
Adds static get styles()
This should be implemented to return an array of styles defined using the `css` tag function. These styles will be applied with `adoptedStyleSheets` where available and shimmed when not.

WIP: needs tests.
  • Loading branch information
Steven Orvell committed Dec 22, 2018
commit 670eb4cf7ba30abf32fd08b0eef0de6eee086d48
20 changes: 9 additions & 11 deletions demo/lit-element.html
Original file line number Diff line number Diff line change
@@ -19,9 +19,13 @@
<ts-element message="Yo" more-info="person"></ts-element>

<script type="module">
import { LitElement, html } from '../lit-element.js';
import { LitElement, html, css } from '../lit-element.js';

class Inner extends LitElement {

static get styles() {
return [css`:host { color: green; }`];
}
render() {
return html`Hello world`;
}
@@ -34,23 +38,13 @@
static get properties() {
return {
nug: {},
zot: {},
foo: {},
bar: {},
whales: {type: Number},
fooBar: {converter: {fromAttribute: parseInt, toAttribute: (value) => value + '-attr'}, reflect: true}
}
}

// a custom getter/setter can be created to customize property processing
get zot() { return this.getAttribute('zot'); }

set zot(value) {
this.setAttribute('zot', value);
this.invalidate();
}


constructor() {
super();
this.foo = 'foo';
@@ -64,6 +58,10 @@
});
}

static get styles() {
return [css`h4 { color: orange;} `];
}

render() {
const {foo, bar, whales, fooBar, nug} = this;
return html`
52 changes: 52 additions & 0 deletions src/lib/css-tag.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
@license
Copyright (c) 2017 The Polymer Project Authors. All rights reserved.
Copy link
Contributor

@kenchris kenchris Dec 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 2019 I assume

This code may only be used under the BSD style license found at http://polymer.github.io/LICENSE.txt
The complete set of authors may be found at http://polymer.github.io/AUTHORS.txt
The complete set of contributors may be found at http://polymer.github.io/CONTRIBUTORS.txt
Code distributed by Google as part of the polymer project is also
subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt
*/
export const supportsAdoptedStyleSheets = ('adoptedStyleSheets' in Document.prototype);

class LiteralString {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would name this CSSLiteral

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


value: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

readonly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


constructor(value: string) {
this.value = value.toString();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the .toString()? If value can be something other than a string, update the type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

}

toString() {
return this.value;
}
}

const literalValue = (value: LiteralString) => {
if (value instanceof LiteralString) {
return value.value;
} else {
throw new Error(
`non-literal value passed to 'css' function: ${value}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to allow user-provided values in safe contexts with context-aware auto-escaping. Not sure how far we can go purely client-side. We may be able to allow directive-like functions that return a sentinel value that parses correctly for a particular context, then replace it, lit-html style. Soy does interesting things here at compile time. Maybe @mikesamuel has ideas.

);
}
};

export const css = (strings: string[], ...values: any[]) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consider having this return an object with an optional .styleSheet and .cssText so that there's a consistent return type for the css tag and so the static style property has a single item type.

This would change below to just:

   const cssText = values.reduce((acc, v, idx) =>
       acc + literalValue(v) + strings[idx + 1], strings[0]);
   return new CSSResult(cssText);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

const cssText = values.reduce((acc, v, idx) =>
acc + literalValue(v) + strings[idx + 1], strings[0]);
let result;
if (supportsAdoptedStyleSheets) {
result = new CSSStyleSheet();
// TODO(sorvell): fix type.
(result as any).replaceSync(cssText);
} else {
result = new LiteralString(cssText);
}
return result;
};

export const cssLiteral = (strings: string[], ...values: any[]) => {
return new LiteralString(values.reduce((acc, v, idx) =>
acc + literalValue(v) + strings[idx + 1], strings[0]));
};
30 changes: 29 additions & 1 deletion src/lib/updating-element.ts
Original file line number Diff line number Diff line change
@@ -11,6 +11,8 @@
* subject to an additional IP rights grant found at
* http://polymer.github.io/PATENTS.txt
*/
import {supportsAdoptedStyleSheets} from './css-tag.js';
export * from './css-tag.js';

/**
* Returns the property descriptor for a property on this prototype by walking
@@ -215,6 +217,14 @@ export abstract class UpdatingElement extends HTMLElement {

static properties: PropertyDeclarations = {};

/**
* Array of styles to apply to the element. The styles should be defined
* using the `css` tag function.
*/
static get styles() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add type of Array<CSSStyleSheet|LiteralString>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added specific type for this and used it here.

return [];
}

/**
* Returns a list of attributes corresponding to the registered properties.
*/
@@ -462,7 +472,25 @@ export abstract class UpdatingElement extends HTMLElement {
* @returns {Element|DocumentFragment} Returns a node into which to render.
*/
protected createRenderRoot(): Element|ShadowRoot {
return this.attachShadow({mode : 'open'});
const shadowRoot = this.attachShadow({mode : 'open'});
const styles = (this.constructor as typeof UpdatingElement).styles;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I (weakly) feel like all this should be in LitElement, since it does the actual rendering.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the shadowRoot rendering (creation + styles) should go together, but I think it's reasonable that this go into LitElement and not UpdatingElement. Let's discuss separately.

if (window.ShadyCSS !== undefined && !(window.ShadyCSS as any).nativeShadow) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment the three cases a little

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// TODO(sorvell): fix type
(window.ShadyCSS as any).prepareAdoptedCssText(styles, this.localName);
} else if (supportsAdoptedStyleSheets) {
// TODO(sorvell): fix type
(shadowRoot as any).adoptedStyleSheets = styles;
} else {
this.requestUpdate();
this._updatePromise.then(() => {
Copy link
Member

@kevinpschaaf kevinpschaaf Jan 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing lit-html to have a render option that allows the user to specify a "before reference" to create the part before would allow us to remove this timing code here and instead put the <style>s in synchronously and just say render(result, this.renderRoot, {beforeRef: this.renderRoot.firstChild})

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since part is cached off the container, avoiding this. However, moved this case to update to avoid the fancy waiting here.

styles.forEach((s) => {
const style = document.createElement('style');
style.textContent = s;
shadowRoot.appendChild(style);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to match Constructible StyleSheet ordering these have to come first in the ShadowRoot. Is that happening because you're directly waiting _updatePromise? Comment if so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, comment added. Note, they come last in the shadowRoot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't they have to come first in case there are <style> tags in the rest of the shadow content?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

});
});
}
return shadowRoot;
}

/**