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

2 way data binding not working #14

Closed
BrandiATMuhkuh opened this Issue Jan 23, 2018 · 16 comments

Comments

Projects
None yet
5 participants
@BrandiATMuhkuh

BrandiATMuhkuh commented Jan 23, 2018

I'm expecting 2 way data binding to work similar to polymer templates but I can't get it to work.
For example using a <paper-toggle-button checked="${toggled}"> only works in one direction (reading, but not writing).

I have also tried <paper-toggle-button checked="[[${toggled}]]"> and <paper-toggle-button checked="[[toggled]]">

@fafcrumb

This comment has been minimized.

Show comment
Hide comment
@fafcrumb

fafcrumb Jan 23, 2018

This is the expected behaviour since you aren't using Polymer's templating system with lit-element, you are using lit-html's, which doesn't support 2-way binding for performance reasons. [[ ]] and {{ }} also don't mean anything to lit-html. The correct way to pass data upwards in lit (and in other one way systems like React), is to use events. In your case, you should listen for on-checked-changed on the paper toggle, and then create a handler that sets the toggled property to the value you get from the event.

fafcrumb commented Jan 23, 2018

This is the expected behaviour since you aren't using Polymer's templating system with lit-element, you are using lit-html's, which doesn't support 2-way binding for performance reasons. [[ ]] and {{ }} also don't mean anything to lit-html. The correct way to pass data upwards in lit (and in other one way systems like React), is to use events. In your case, you should listen for on-checked-changed on the paper toggle, and then create a handler that sets the toggled property to the value you get from the event.

@BrandiATMuhkuh

This comment has been minimized.

Show comment
Hide comment
@BrandiATMuhkuh

BrandiATMuhkuh Jan 24, 2018

thank you that's good to know. I was expecting '[[ ]]' to work because this element is part of PolymerLabs. However, since polymer 3 is completely written in JS it's a much better approach to use listeners :)

BrandiATMuhkuh commented Jan 24, 2018

thank you that's good to know. I was expecting '[[ ]]' to work because this element is part of PolymerLabs. However, since polymer 3 is completely written in JS it's a much better approach to use listeners :)

@BrandiATMuhkuh

This comment has been minimized.

Show comment
Hide comment
@BrandiATMuhkuh

BrandiATMuhkuh Jan 24, 2018

I tried the approach with the event listener but it seems to not be able to keep the right scope. Which means, the this word in the event listener references the element that created the event, but not the polymer element.
I have also tried to add the event listener to the render({toggled, onCheckedChanged} but no luck.

import { LitElement, html } from '../node_modules/@polymer/lit-element/lit-element.js'
import "../node_modules/@polymer/paper-toggle-button/paper-toggle-button.js";

export class TestPageTwo extends LitElement {

  constructor() {
    super();
    this.toggled = false;
  }

  // properties, observers, etc. are identical to 2.x
  static get properties() {
    return {
      toggled: Boolean
    }

  }

  onCheckedChanged(e) {
    this.set("toggled", e.detail.value);
  }

  render({toggled, onCheckedChanged}) {
    return html`
    ${toggled}
    <paper-toggle-button checked="${toggled}" on-checked-changed="${this.onCheckedChanged}"></paper-toggle-button>
     `;
  }
}

customElements.define('test-page-two', TestPageTwo);

BrandiATMuhkuh commented Jan 24, 2018

I tried the approach with the event listener but it seems to not be able to keep the right scope. Which means, the this word in the event listener references the element that created the event, but not the polymer element.
I have also tried to add the event listener to the render({toggled, onCheckedChanged} but no luck.

import { LitElement, html } from '../node_modules/@polymer/lit-element/lit-element.js'
import "../node_modules/@polymer/paper-toggle-button/paper-toggle-button.js";

export class TestPageTwo extends LitElement {

  constructor() {
    super();
    this.toggled = false;
  }

  // properties, observers, etc. are identical to 2.x
  static get properties() {
    return {
      toggled: Boolean
    }

  }

  onCheckedChanged(e) {
    this.set("toggled", e.detail.value);
  }

  render({toggled, onCheckedChanged}) {
    return html`
    ${toggled}
    <paper-toggle-button checked="${toggled}" on-checked-changed="${this.onCheckedChanged}"></paper-toggle-button>
     `;
  }
}

customElements.define('test-page-two', TestPageTwo);
@kenchris

This comment has been minimized.

Show comment
Hide comment
@kenchris

kenchris Jan 24, 2018

Contributor
<paper-toggle-button checked="${toggled}" on-checked-changed="${this.onCheckedChanged.bind(this)}"></paper-toggle-button>
Contributor

kenchris commented Jan 24, 2018

<paper-toggle-button checked="${toggled}" on-checked-changed="${this.onCheckedChanged.bind(this)}"></paper-toggle-button>
@kenchris

This comment has been minimized.

Show comment
Hide comment
@kenchris

kenchris Jan 24, 2018

Contributor

Just bind it either before or when you attach it.

"this" is supposed to be the element on which the event is fired... so paper-toggle-button, so if you don't want that, rebind it.

You could also do it in the constructor

  this.onCheckedChanged = this.onCheckedChanged.bind(this)
Contributor

kenchris commented Jan 24, 2018

Just bind it either before or when you attach it.

"this" is supposed to be the element on which the event is fired... so paper-toggle-button, so if you don't want that, rebind it.

You could also do it in the constructor

  this.onCheckedChanged = this.onCheckedChanged.bind(this)
@BrandiATMuhkuh

This comment has been minimized.

Show comment
Hide comment
@BrandiATMuhkuh

BrandiATMuhkuh Jan 24, 2018

Wonderful. Thanks you. I thought I need to you .bind(this), but didn't know how. Now it works.
Q: Will the API for that change in the future to automate the .bind() process?

BrandiATMuhkuh commented Jan 24, 2018

Wonderful. Thanks you. I thought I need to you .bind(this), but didn't know how. Now it works.
Q: Will the API for that change in the future to automate the .bind() process?

@BrandiATMuhkuh

This comment has been minimized.

Show comment
Hide comment
@BrandiATMuhkuh

BrandiATMuhkuh Jan 24, 2018

For completion. Here is the working element

import { LitElement, html } from '../node_modules/@polymer/lit-element/lit-element.js'
import "../node_modules/@polymer/paper-toggle-button/paper-toggle-button.js";

export class TestPageTwo extends LitElement {

  constructor() {
    super();
    this.toggled = false;
  }

  // properties, observers, etc. are identical to 2.x
  static get properties() {
    return {
      toggled: Boolean
    }
  }

  onCheckedChanged(e) {
    this.toggled = e.detail.value;
  }

  render({toggled}) {
    return html`
    ${toggled}
    <paper-toggle-button checked="${toggled}" on-checked-changed="${this.onCheckedChanged.bind(this)}"></paper-toggle-button>
     `;
  }
}

customElements.define('test-page-two', TestPageTwo);

BrandiATMuhkuh commented Jan 24, 2018

For completion. Here is the working element

import { LitElement, html } from '../node_modules/@polymer/lit-element/lit-element.js'
import "../node_modules/@polymer/paper-toggle-button/paper-toggle-button.js";

export class TestPageTwo extends LitElement {

  constructor() {
    super();
    this.toggled = false;
  }

  // properties, observers, etc. are identical to 2.x
  static get properties() {
    return {
      toggled: Boolean
    }
  }

  onCheckedChanged(e) {
    this.toggled = e.detail.value;
  }

  render({toggled}) {
    return html`
    ${toggled}
    <paper-toggle-button checked="${toggled}" on-checked-changed="${this.onCheckedChanged.bind(this)}"></paper-toggle-button>
     `;
  }
}

customElements.define('test-page-two', TestPageTwo);

@FluorescentHallucinogen

This comment has been minimized.

Show comment
Hide comment
@FluorescentHallucinogen

FluorescentHallucinogen May 14, 2018

Contributor

@BrandiATMuhkuh I believe _render should be used instead of render in your last code. 😉

Contributor

FluorescentHallucinogen commented May 14, 2018

@BrandiATMuhkuh I believe _render should be used instead of render in your last code. 😉

@FluorescentHallucinogen

This comment has been minimized.

Show comment
Hide comment
@FluorescentHallucinogen

FluorescentHallucinogen May 16, 2018

Contributor

But what if I want to use <mwc-switch> (which is also based on lit-element) instead of <paper-toggle-button>?

The problem is that <mwc-switch> has no the checked-changed custom event unlike <paper-toggle-button>.

@azakus @sorvell PTAL.

Contributor

FluorescentHallucinogen commented May 16, 2018

But what if I want to use <mwc-switch> (which is also based on lit-element) instead of <paper-toggle-button>?

The problem is that <mwc-switch> has no the checked-changed custom event unlike <paper-toggle-button>.

@azakus @sorvell PTAL.

@FluorescentHallucinogen

This comment has been minimized.

Show comment
Hide comment
@FluorescentHallucinogen

FluorescentHallucinogen May 16, 2018

Contributor

What about something like Polymer's notify (automatic property change notification events) to make upward data flow easier?

Contributor

FluorescentHallucinogen commented May 16, 2018

What about something like Polymer's notify (automatic property change notification events) to make upward data flow easier?

@web-padawan

This comment has been minimized.

Show comment
Hide comment
@web-padawan

web-padawan May 16, 2018

Contributor

@FluorescentHallucinogen this is the new reality, where no upward data flow is supposed to be always kept in mind. Still the Web Component should produce events, but I guess it would make sense to make the switch component more aligned with the native input and produce a change event.

Contributor

web-padawan commented May 16, 2018

@FluorescentHallucinogen this is the new reality, where no upward data flow is supposed to be always kept in mind. Still the Web Component should produce events, but I guess it would make sense to make the switch component more aligned with the native input and produce a change event.

@FluorescentHallucinogen

This comment has been minimized.

Show comment
Hide comment
@FluorescentHallucinogen

FluorescentHallucinogen May 16, 2018

Contributor

@web-padawan Could you please explain what's wrong with two-way data binding (upward data flow)? Two-way data binding was (is) my favorite feature of Polymer. What are the advantages of unidirectional data flow?

Contributor

FluorescentHallucinogen commented May 16, 2018

@web-padawan Could you please explain what's wrong with two-way data binding (upward data flow)? Two-way data binding was (is) my favorite feature of Polymer. What are the advantages of unidirectional data flow?

@FluorescentHallucinogen

This comment has been minimized.

Show comment
Hide comment
@FluorescentHallucinogen

FluorescentHallucinogen May 20, 2018

Contributor

I've found https://github.com/DiiLord/lit-element and it has notify.

@justinfagnani @DiiLord What about to port it to this element?

Contributor

FluorescentHallucinogen commented May 20, 2018

I've found https://github.com/DiiLord/lit-element and it has notify.

@justinfagnani @DiiLord What about to port it to this element?

@web-padawan

This comment has been minimized.

Show comment
Hide comment
@web-padawan

web-padawan May 20, 2018

Contributor

Welcome to 2018, everyone is writing his own lit-element implementation 😀

Contributor

web-padawan commented May 20, 2018

Welcome to 2018, everyone is writing his own lit-element implementation 😀

@FluorescentHallucinogen

This comment has been minimized.

Show comment
Hide comment
@FluorescentHallucinogen

FluorescentHallucinogen May 22, 2018

Contributor

lit-html-brackets also looks very interesting.

Contributor

FluorescentHallucinogen commented May 22, 2018

lit-html-brackets also looks very interesting.

@FluorescentHallucinogen

This comment has been minimized.

Show comment
Hide comment
@FluorescentHallucinogen

FluorescentHallucinogen May 22, 2018

Contributor

HTML Template Instantiation proposal also adopts mustache syntax as standardized template language.

Contributor

FluorescentHallucinogen commented May 22, 2018

HTML Template Instantiation proposal also adopts mustache syntax as standardized template language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment