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

DOM templating update. #2500

Merged
merged 3 commits into from
Mar 30, 2018
Merged

DOM templating update. #2500

merged 3 commits into from
Mar 30, 2018

Conversation

arthurevans
Copy link

@arthurevans arthurevans commented Mar 18, 2018

No description provided.

@arthurevans arthurevans requested a review from a user March 18, 2018 19:41
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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


The DOM template is also processed to enable data binding and declarative event handlers.

## Specify a DOM template

Polymer provides three basic ways to specify a DOM template:
To specify a template, defining a `template` property on the constructor. When extending an
Copy link

Choose a reason for hiding this comment

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

defining -> define

Copy link

Choose a reason for hiding this comment

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

Apologies in advance for any/most of these being things that I introduced when I updated this chapter... XD

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.


### Specify a template using dom-module
To specif the element's template, define a `template` property on the element's
Copy link

Choose a reason for hiding this comment

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

specif -> specify

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@ghost
Copy link

ghost commented Mar 26, 2018

oh crud I submitted the review - haven't finished it yet. pls disregard my review until further notice

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

2nd of 2 reviews because I accidentally submitted the first one


To specify an element's DOM template using a `<dom-module>`:
The template getter must return an instance of `HTMLTemplateElement`. Use the `html` helper function
to generate an `HTMLTemplateElement` instance from a JavaScript template literal.
Copy link

Choose a reason for hiding this comment

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

"Use the html helper function to generate..."

Do we need to say where to import it from?

"Import the html helper function from polymer-element.js and use it to generate..."

Copy link
Author

Choose a reason for hiding this comment

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

It's in the sample, but I added a parenthetical here so as not to make the sentence any more complicated than it was... WDYT?

Copy link

Choose a reason for hiding this comment

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

seems fine to me

customElements.define('child-class', ChildClass);
```

[See a working example in Plunker](http://plnkr.co/edit/vS99al?p=preview).
Copy link

Choose a reason for hiding this comment

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

http://plnkr.co/edit/vS99al?p=preview Seems to be a 2.0 sample

Copy link
Author

Choose a reason for hiding this comment

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

Replaced w/ correct sample. (I hope)

```js
import {PolymerElement, html} from '@polymer/polymer/polymer-element.js';

export class BaseClass extends Polymer.Element {
Copy link

Choose a reason for hiding this comment

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

export class BaseClass extends Polymer.Element {

->

export class BaseClass extends PolymerElement {

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

return `<style>:host { color: blue; }</style>
<h2>String template</h2>
<div>I've got a string template!</div>`
export class BaseClass extends Polymer.Element {
Copy link

Choose a reason for hiding this comment

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

export class BaseClass extends Polymer.Element {

->

export class BaseClass extends PolymerElement {

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

* The `html` function checks each expression value. If the expression is an
`HTMLTemplateElement`, the `innerHTML` of the template is interpolated.
* To protect against XSS vulnerabilities, `html` only interpolates `HTMLTemplateElement`
values or template literals tagged with `htmlLiteral`. For information on usign `htmlLiteral` to
Copy link

Choose a reason for hiding this comment

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

usign -> using

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

static get template() {
return null;
}
```

### URLs in templates {#urls-in-templates}
Copy link

Choose a reason for hiding this comment

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

URLs in templates

I think some text needs updating in this section - it references HTML Imports

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, this is still sort of up in the air. I'll add a note here that the section needs to be updated when Polymer/polymer#5163 gets resolved.

@@ -226,12 +333,14 @@ Example: { .caption }
<dom-module id="x-custom">
Copy link

Choose a reason for hiding this comment

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

I think we just need the updated script, can html tags be removed?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, fixed.

@@ -226,12 +333,14 @@ Example: { .caption }
<dom-module id="x-custom">

<template>
Hello World from <span id="name"></span>!

</template>

<script>
class MyElement extends Polymer.Element {
Copy link

Choose a reason for hiding this comment

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

class MyElement extends Polymer.Element {

->

class MyElement extends PolymerElement {

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

@@ -251,6 +360,9 @@ use the standard DOM `querySelector` method.

## Remove empty text nodes {#strip-whitespace}
Copy link

Choose a reason for hiding this comment

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

Skipping review of this section

@@ -338,14 +454,16 @@ This is a fairly rare use case.
```html
<dom-module id="custom-template">
Copy link

Choose a reason for hiding this comment

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

As before, can remove html stuff?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, fixed.

@arthurevans
Copy link
Author

Updated, PTAL.

@arthurevans arthurevans merged commit f999d89 into master Mar 30, 2018
@arthurevans arthurevans deleted the template branch March 30, 2018 17:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants