vaadin / vaadin-button-flow

Vaadin Flow Java API for vaadin/vaadin-button Web Component
https://vaadin.com/components/vaadin-button
Other
0 stars 7 forks source link

Calling setText for Button appends the text to an existing caption on MS Edge #120

Closed tohkawaics closed 4 years ago

tohkawaics commented 5 years ago

The bug #43 still seems to persist on MS Edge.

ExampleView.java:

@Route("example")
@Tag("example-template")
@HtmlImport("frontend://templates/example-template.html")
public class ExampleView extends PolymerTemplate<TemplateModel> {

  @Id("btnExample")
  private Button btnExample;

  public ExampleView() {
    btnExample.setText("test");
  }

}

example-template.html:

<dom-module id="example-template">
    <template>
        <vaadin-button id="btnExample">example</vaadin-button>
    </template>

    <script>
        class ExampleTemplate extends Polymer.Element {
            static get is() {
                return 'example-template'
            }
        }
        customElements.define(ExampleTemplate.is, ExampleTemplate);

    </script>
</dom-module>

With Chrome, no problem. chrome

With MS Edge, the problem persists. edge

I tried both 12.0.6 and 13.0.0.beta1.

Haprog commented 5 years ago

This seems to be some kind of a timing issue. And since it happens in Edge but not in Chrome I guess it might be related to Shady DOM polyfill.

button.setText("test") seems to work also in Edge if it's triggered later (e.g. after clicking some button) but for some reason fails in Edge when it's done immediately in the view constructor of the PolymerTemplate.

I'm not sure now if this is something that should be actually fixed in Flow itself, in here (vaadin-button-flow) or somewhere else.

I figured out that the issue doesn't happen if the operation is delayed by just a JS setTimeout() of 0 duration or by just calling Polymer.flush() in JS before setting the contents.

This problem seems to happen also without Flow in just a plain frontend Polymer app if you have a Polymer template with a <vaadin-button> and try to override its textContent in the ready() method (e.g. like this.$.btnExample.textContent = 'test';). This can also be made to work just by running Polymer.flush() first. However if using e.g. native <button> instead, there is no problem even in Edge without flush or timeout so this is probably related to shadow DOM and maybe only if the component uses <slot> though I didn't test this.

Here are two alternatives to workaround the issue when using PolymerTemplate with Flow.

  1. instead of:

    btnExample.setText("test");

    do:

    btnExample.getElement().executeJavaScript("Polymer.flush(); this.textContent = $0;", "test");
  2. add this to your ExampleTemplate class in example-template.html:

    ready() {
     super.ready();
     Polymer.flush();
    }

    and in the Java side just use btnExample.setText("test"); as normal.

Haprog commented 5 years ago

If someone wants to investigate this a bit more, here's a minimal Polymer 2 app to reproduce the issue in Edge

You can use polymer init CLI tool to init a basic Polymer 2 application with a tag name my-app and just replace the contents of my-app.html with this:

<link rel="import" href="../../bower_components/polymer/polymer-element.html">

<script>
  const html = Polymer.html;
  class MyElement extends Polymer.Element {
    static get is() { return 'my-element'; }
    static get template() { return html`<strong><slot></slot></strong>`; }
  }
  window.customElements.define(MyElement.is, MyElement);

  class MyApp extends Polymer.Element {
    static get is() { return 'my-app'; }
    static get template() { return html`<my-element id="testElement">example</my-element>`; }
    ready() {
      super.ready();
      //Polymer.flush(); // Uncomment this line to fix the problem in Edge
      this.$.testElement.textContent = 'test';
    }
  }

  window.customElements.define(MyApp.is, MyApp);
</script>
Haprog commented 5 years ago

This is probably not a big issue in context of Flow though since it only happens if you have contents for the element in the HTML template and immediately override it from Java. You should probably set the initial content only in one place (in this case in the HTML template).

Everything should still work fine if the setText() is used to override it from Java later as triggered by some event or user action after the page has loaded.

Haprog commented 5 years ago

Actually I just noticed that window.ShadyDOM.flush() also fixes this. So maybe better would be to run if (window.ShadyDOM) window.ShadyDOM.flush();. Though I'm not sure if ShadyDOM.flush() does more or less stuff than Polymer.flush().

shadikhani commented 4 years ago

I worked on this issue and found out that it uses a removeAll method in the setText that all the children nodes are removed there, all the "slot" attributes are deleted. In Edge, there is no shadow DOM so the text remains and also in the Java code, I have no access to the text to remove it. I tried to implement this issue in another component. I used vaadin-checkbox-flow and I saw this problem does not exist in this component. I compared the Java code of the two components and found them alike, then I compared the web components code and notice that there were some differences there. I used the check-box code this._updateLabelAttribute() and tested again and it worked fine.