vaadin / web-components

A set of high-quality standards based web components for enterprise web applications. Part of Vaadin 20+
https://vaadin.com/docs/latest/components
457 stars 83 forks source link

Dialog contents should never be connected inside `<vaadin-dialog>` #572

Open Artur- opened 3 years ago

Artur- commented 3 years ago

Given

<vaadin-dialog no-close-on-outside-click no-close-on-esc draggable>
  <template>
    <entity-editor></entity-editor>
  </template>
</vaadin-dialog>

and

@customElement('entity-editor')
export class EntityEditor extends LitElement {
  connectedCallback() {
    super.connectedCallback();
    if (!this.hasAttribute('instance')) this.setAttribute('instance', '' + EntityEditor.instance);
    EntityEditor.instance++;
    console.log('Connected', this);
  }
  disconnectedCallback() {
    super.disconnectedCallback();
    console.log('Disconnected', this);
  }

You will see the following in the log when opening the dialog

Connected <entity-editor instance=​"1">​…​</entity-editor>
Disconnected <entity-editor instance=​"1">​…​</entity-editor>
Connected <entity-editor instance=​"1">​…​</entity-editor>

and the following when closing it

Disconnected <entity-editor instance=​"1">​…​</entity-editor>
Connected <entity-editor instance=​"1">​…​</entity-editor>

As connectedCallback is called after closing the dialog, any attempts to e.g. register global listeners in connectedCallback and removing them in disconnectedCallback will fail and the listeners will remain after the dialog is closed.

The reason for the extra events is that the <entity-editor> element is first created inside <vaadin-dialog><vaadin-dialog-overlay> and then <vaadin-dialog-overlay> is moved to the <body> element. After closing the dialog, the <vaadin-dialog-overlay> element is moved back inside <vaadin-dialog>.

It seems like the correct solution would be that <vaadin-dialog-overlay> is never attached to <vaadin-dialog> in the first place, nor moved back there.

Workaround:

  connectedCallback() {
    super.connectedCallback();
    if (this.parentElement?.parentElement == document.body) {
      // This is a real connect
      this.connected = true;
      document.body.addEventListener(...)
    }
  }
  disconnectedCallback() {
    super.disconnectedCallback();
    if (this.connected) {
      // This is a real disconnect
      this.connected = false;
      document.body.removeEventListener(...)
    }
  }
web-padawan commented 3 years ago

See https://github.com/vaadin/web-components/issues/267#issuecomment-828261111 for a workaround using renderer function.