Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

3.0-ify shadow-dom, style-shadow-dom, and custom-css-properties #2476

Merged
merged 8 commits into from
Apr 29, 2018

Conversation

ghost
Copy link

@ghost ghost commented Feb 20, 2018

```html
<link rel="import" href="/bower_components/polymer/lib/utils/flattened-nodes-observer.html">
```js
import * from '@polymer/polymer/utils/flattened-nodes-observer.js';
Copy link
Author

Choose a reason for hiding this comment

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

check this

Choose a reason for hiding this comment

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

I don't think this is a valid format.

If the module has a default export (most of ours don't), you can do:

import defaultExport from "module specifier"

To create a new object that contains references to all of the exports from a module, you can do:

import * as myNameforFNO from '@Polymer.../flattened-nodes-observer.js';

Then use myNameforFNO.FlattenedNodesObserver;

Or you could just import individual symbols exported from the module:

import {FlattenedNodesObserver} from 'module specifier';

@ghost ghost changed the title update shadow dom docs for p3 - first pass update shadow dom docs for p3 - ready for review Feb 28, 2018
@ghost
Copy link
Author

ghost commented Feb 28, 2018

probably needs some iteration but ready for a look @arthurevans https://p3-shadow-dom-concepts-dot-polymer-project.appspot.com/

@ghost ghost changed the title update shadow dom docs for p3 - ready for review 3.0-ify devguide/shadow-dom, devguide/style-shadow-dom, devguide/custom-css-properties Mar 13, 2018
@ghost ghost changed the title 3.0-ify devguide/shadow-dom, devguide/style-shadow-dom, devguide/custom-css-properties 3.0-ify devguide/shadow-dom, style-shadow-dom, custom-css-properties Mar 13, 2018
@ghost ghost changed the title 3.0-ify devguide/shadow-dom, style-shadow-dom, custom-css-properties 3.0-ify shadow-dom, style-shadow-dom, and custom-css-properties Mar 13, 2018
@ghost ghost added the 3.0 update label Mar 13, 2018
@ghost ghost requested a review from arthurevans March 30, 2018 22:06
@ghost ghost assigned arthurevans Mar 30, 2018
Copy link

@arthurevans arthurevans left a comment

Choose a reason for hiding this comment

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

Left some notes. Sorry these are so late--forgot to submit earlier.

<!-- import the custom-style polyfill to prevent "leaks" in some browsers -->
<script type="module" src="./node_modules/@polymer/polymer/lib/elements/custom-style.js"></script>

<!-- wrap document-level styles to avoid "leaks" in some browsers -->

Choose a reason for hiding this comment

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

I think this comment is misleading. Here you're using the <custom-style> to ensure that the custom properties are shimmed (polyfilled) on browsers that don't support them natively. I don't think leaking is an issue here (maybe @azakus knows better)...

```

## Create custom properties
If you provide documentation for the custom properties your element provides, users won't need to know any implementation details. See [Documenting your elements](/{{{polymer_version_dir}}}/docs/tools/documentation) for more information, or take a look at the [documentation for the Polymer `paper-ui-elements`](https://www.webcomponents.org/collection/PolymerElements/paper-ui-elements) for examples.

Choose a reason for hiding this comment

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

"won't" => "don't" real tiny nit here, but should probably be present tense, for simplicity.

<!-- import the custom-style polyfill -->
<script type="module" src="node_modules/@polymer/polymer/lib/elements/custom-style.js"></script>

<!-- wrap document-level styles with the custom-style polyfill to prevent style "leak" in some browsers -->

Choose a reason for hiding this comment

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

nit: custom-style isn't a polyfill, it's an element that makes sure the contained styles get polyfilled.

As above, it may be more relevant that the custom-style element invokes the custom properties polyfill, which polyfills custom property support on browsers that don't have it (as well as the shady scoping polyfill, which prevents style leaks, as described).

Don't need the is="custom-style" anymore--3.x code can't run in 1.x, so we don't need hybrid syntax.


<!-- wrap document-level styles to avoid "leaks" in some browsers -->
<custom-style>
<style is="custom-style">

Choose a reason for hiding this comment

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

Shouldn't need the 'is="custom-style" anymore.

```html
...
<custom-style>
<style is="custom-style">

Choose a reason for hiding this comment

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

No is="custom-style"

```

[See it on Plunker](http://plnkr.co/edit/hR3I4w?p=preview)

## Share styles between elements

### Use style modules {#style-modules}

The preferred way to share styles is with *style modules*. You can package up styles in a style module, and share them between elements.

Choose a reason for hiding this comment

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

Going forward, we're probably going to recommend using template mechanism rather than style modules. We should add that as a separate update after this one, though, I think.

Copy link
Author

Choose a reason for hiding this comment

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

ok, created #2513 to track

</template>
</dom-module>
```js
static template get() {

Choose a reason for hiding this comment

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

static get template()

```

## Use `custom-style` in document-level styles {#custom-style}
`custom-style` is not included in the `@polymer/polymer/polymer-element.js` package. Import `custom-style` from `@polymer/polymer/lib/elements/custom-style.js`:

Choose a reason for hiding this comment

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

package => module

<body>
<custom-element></custom-element>
</body>
```

*Note: You should only use `custom-style` to define styles for the main document. To define styles for an element's local DOM, just use a `<style>` block.*

Choose a reason for hiding this comment

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

Fix note formatting: Only bold run-in head, add {.alert .alert-info} ... Extra credit, replace Note with a more informative run-in so people know whether they should read it. Like:

Don't use custom-style inside an element's template.

also: local DOM => shadow DOM


*Note: You should only use `custom-style` to define styles for the main document. To define styles for an element's local DOM, just use a `<style>` block.*

### Examples

In the first code sample, the style for the `p` element “leaks into Paragraph B in browsers that haven’t implemented the Shadow DOM v1 specs. In the second code sample, the developer has used `custom-style` to wrap the style block, preventing this leak.
In the following code sample, the document-level style in index.html "leaks" into the shadow DOM of `<custom-element>` in browsers that haven’t implemented the Shadow DOM v1 specs.

Choose a reason for hiding this comment

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

Note for the future: at some point (not right now) we should clarify what we mean by "leaking." It could be confusing, because inheritance is also a form of "leaking," right?

Specifically, what we mean when we say styles can't leak is that _a selector in one scope (i.e., in the main document) can't match an element in a different scope (like in a shadow root)... Except in a couple of very specific instances, namely :host and ::slotted() ...

Anyway, think on it... We should convey that at some point.

Copy link
Author

@ghost ghost Apr 12, 2018

Choose a reason for hiding this comment

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

agree, opened #2514

@ghost
Copy link
Author

ghost commented Apr 13, 2018

@arthurevans Thanks! Incorporated feedback

@ghost
Copy link
Author

ghost commented Apr 28, 2018

@arthurevans would you mind taking another look - there were a couple of missing parts d9d4ff6

@arthurevans
Copy link

Reviewed added parts, LGTM. Merging.

@arthurevans arthurevans merged commit 22b8531 into master Apr 29, 2018
@arthurevans arthurevans deleted the p3-shadow-dom-concepts branch April 29, 2018 19:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants