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.

Sign up for GitHub

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

Two small fixes for the styles doc #535

Merged
merged 4 commits into from
Dec 16, 2021
Merged

Two small fixes for the styles doc #535

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87207c3
Document octal escape sequence issue. Fixes #414.
Sep 30, 2021
bbd9028
Add note on super.styles. Fixes #475.
Sep 30, 2021
de60a1c
Merge branch 'main' into style-doc-fixes
Dec 7, 2021
8e26555
Recommend against super.styles in TypeScript.
Dec 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions packages/lit-dev-content/site/docs/components/styles.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ The value of the static `styles` class field can be:

The static `styles` class field is _almost always_ the best way to add styles to your component, but there are some use cases you can't handle this way—for example, customizing styles per instance. For alternate ways to add styles, see [Defining scoped styles in the template](#styles-in-the-template).


### Using expressions in static styles {#expressions}

Static styles apply to all instances of a component. Any expressions in CSS are evaluated **once**, then reused for all instances.
Expand Down Expand Up @@ -74,6 +75,12 @@ Using an array of tagged template literals, a component can inherit the styles f

{% playground-ide "docs/components/style/superstyles" %}

<div class="alert alert-info">

**Using super.styles**. You can also use `super.styles` to reference the superclass's styles property, but TypeScript doesn't convert this correctly when compiling for ES5. Explicitly referencing the superclass, as shown in the example, avoids this issue.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe clarify that this does work for ES2015 and above, though. Also I think we should show the super.styles approach in the examples. (@AndrewJakubowicz found that it works fine in TypeScript 4.4, we just need to update the Playground's TypeScript version).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, cool. Yeah, I agree that showing the super.styles approach as the default is preferable. Is there a bug for updating the Playground typescript version?

Copy link
Member

Choose a reason for hiding this comment

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

google/playground-elements#217 to release with the new version. After this I'll send a PR to update lit.dev. Should be able to get that in before this PR merges.

Copy link
Member

Choose a reason for hiding this comment

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

#537

Copy link
Member

Choose a reason for hiding this comment

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

Huh, bumping the TypeScript version fixed the type error, so there's no longer a squiggly when doing super.styles, but it didn't fix the output, which is still invalid note the (void 0).styles:

var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
import { css } from './node_modules/lit@2.0.0/index.js';
import { customElement } from './node_modules/lit@2.0.0/decorators.js';
import { SuperElement } from './super-element.js';
let MyElement = class MyElement extends SuperElement {
};
MyElement.styles = [
    (void 0).styles,
    css `div {
      color: red;
    }`
];
MyElement = __decorate([
    customElement('my-element')
], MyElement);
export { MyElement };

I wonder if we need to tweak the compiler options (https://github.com/google/playground-elements/blob/05538da0ee6fe13b3345d89b805dd06b2b8f30b3/src/typescript-worker/language-service-context.ts#L9)

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, the latest update seems fine (though maybe we should note it's also broken for ES2015+ with a class decorator, not just ES5?) I think we should file a typescript bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think under those circumstances we should just recommend against using it in TypeScript. If someone uses it and subsequently decides to add an IE11 build or to adopt decorators, their code will break and that seems bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... especially since we invariably show TypeScript with decorators, so the only circumstance where it works is off the well-lit path.

Copy link
Contributor

Choose a reason for hiding this comment

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

Made a repro. Can look for an open issue in TypeScript or open a new one.

Copy link
Contributor

@AndrewJakubowicz AndrewJakubowicz Dec 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Another maybe interesting note, if you set useDefineForClassFields in the previous repro then the (void 0) bug no longer occurs.


</div>

### Sharing styles

You can share styles between components by creating a module that exports tagged styles:
Expand Down Expand Up @@ -105,6 +112,22 @@ class MyElement extends LitElement {
}
```

### Using unicode escapes in styles

CSS's unicode escape sequence is a backslash followed by four or six hex digits: for example, `\2022` for a bullet character. This similar to the format of JavaScript's deprecated _octal_ escape sequences, so using these sequences in a `css` tagged template literal causes an error.

There are two work-arounds for adding a unicode escape to your styles:

* Add a second backslash (for example, `\\2022`).
* Use the JavaScript escape sequence, starting with `\u` (for example, `\u2022`).
Copy link
Contributor

Choose a reason for hiding this comment

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

The third is to just include the unicode character in the JS source.


```js
static styles = css`
div::before {
content: '\u2022';
}
```

## Shadow DOM styling overview {#shadow-dom}

This section gives a brief overview of shadow DOM styling.
Expand Down