fix: avoid opened changed events when moving dialog within DOM (#5995) (

#5999) * fix: avoid opened changed events when moving dialog within DOM * explicitly close confirm-dialog in tests before opening new one Co-authored-by: Sascha Ißbrücker <[email protected]>
vaadin · Jun 20, 2023 · ec3fd1b · ec3fd1b
1 parent 52a1201
commit ec3fd1b
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 8 deletions.
diff --git a/packages/confirm-dialog/test/confirm-dialog.test.js b/packages/confirm-dialog/test/confirm-dialog.test.js
@@ -548,6 +548,10 @@ describe('vaadin-confirm-dialog', () => {
         await oneEvent(overlay, 'vaadin-overlay-open');
       });
 
+      afterEach(() => {
+        confirm.opened = false;
+      });
+
       it('should update width after opening the dialog', () => {
         confirm._contentWidth = '300px';
         expect(getComputedStyle(overlay.$.overlay).width).to.be.equal('300px');

diff --git a/packages/dialog/src/vaadin-dialog.js b/packages/dialog/src/vaadin-dialog.js
@@ -269,9 +269,15 @@ class Dialog extends OverlayClassMixin(
   /** @protected */
   disconnectedCallback() {
     super.disconnectedCallback();
-    // Close overlay and memorize opened state
-    this.__restoreOpened = this.opened;
-    this.opened = false;
+    // Automatically close the overlay when dialog is removed from DOM
+    // Using a timeout to avoid toggling opened state, and dispatching change
+    // events, when just moving the dialog in the DOM
+    setTimeout(() => {
+      if (!this.isConnected) {
+        this.__restoreOpened = this.opened;
+        this.opened = false;
+      }
+    });
   }
 
   /** @private */

diff --git a/packages/dialog/test/dialog.test.js b/packages/dialog/test/dialog.test.js
@@ -1,5 +1,6 @@
 import { expect } from '@esm-bundle/chai';
 import { aTimeout, esc, fixtureSync, oneEvent } from '@vaadin/testing-helpers';
+import sinon from 'sinon';
 import '../vaadin-dialog.js';
 import { getDeepActiveElement } from '@vaadin/a11y-base/src/focus-utils.js';
 
@@ -113,19 +114,44 @@ describe('vaadin-dialog', () => {
       });
     });
 
-    describe('detaching', () => {
-      it('should close the overlay when detached', () => {
-        dialog.parentNode.removeChild(dialog);
+    describe('removing and adding to the DOM', () => {
+      it('should close the overlay when removed from DOM', async () => {
+        dialog.remove();
+        await aTimeout(0);
+
+        expect(dialog.opened).to.be.false;
+      });
+
+      it('should restore opened state when added to the DOM', async () => {
+        const parent = dialog.parentNode;
+        dialog.remove();
+        await aTimeout(0);
         expect(dialog.opened).to.be.false;
+
+        parent.appendChild(dialog);
+        expect(dialog.opened).to.be.true;
       });
 
-      it('should not close the overlay when moved within the DOM', () => {
+      it('should not close the overlay when moved within the DOM', async () => {
         const newParent = document.createElement('div');
         document.body.appendChild(newParent);
-
         newParent.appendChild(dialog);
+        await aTimeout(0);
+
         expect(dialog.opened).to.be.true;
       });
+
+      it('should not dispatch opened changed events when moved within the DOM', async () => {
+        const onOpenedChanged = sinon.spy();
+        dialog.addEventListener('opened-changed', onOpenedChanged);
+
+        const newParent = document.createElement('div');
+        document.body.appendChild(newParent);
+        newParent.appendChild(dialog);
+        await aTimeout(0);
+
+        expect(onOpenedChanged.called).to.be.false;
+      });
     });
 
     describe('modeless', () => {