fix: ignore target set while popover is detached until re-attached (#…

…8350)
vaadin · Dec 16, 2024 · b402736 · b402736
1 parent a413851
commit b402736
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 11 deletions.
diff --git a/packages/popover/src/vaadin-popover-target-mixin.js b/packages/popover/src/vaadin-popover-target-mixin.js
@@ -34,27 +34,32 @@ export const PopoverTargetMixin = (superClass) =>
          */
         target: {
           type: Object,
-          observer: '__targetChanged',
+        },
+
+        /** @private */
+        __isConnected: {
+          type: Boolean,
+          sync: true,
         },
       };
     }
 
+    static get observers() {
+      return ['__targetOrConnectedChanged(target, __isConnected)'];
+    }
+
     /** @protected */
     connectedCallback() {
       super.connectedCallback();
 
-      if (this.target) {
-        this._addTargetListeners(this.target);
-      }
+      this.__isConnected = true;
     }
 
     /** @protected */
     disconnectedCallback() {
       super.disconnectedCallback();
 
-      if (this.target) {
-        this._removeTargetListeners(this.target);
-      }
+      this.__isConnected = false;
     }
 
     /** @private */
@@ -82,14 +87,16 @@ export const PopoverTargetMixin = (superClass) =>
     }
 
     /** @private */
-    __targetChanged(target, oldTarget) {
-      if (oldTarget) {
-        this._removeTargetListeners(oldTarget);
+    __targetOrConnectedChanged(target, isConnected) {
+      if (this.__previousTarget && (this.__previousTarget !== target || !isConnected)) {
+        this._removeTargetListeners(this.__previousTarget);
       }
 
-      if (target) {
+      if (target && isConnected) {
         this._addTargetListeners(target);
       }
+
+      this.__previousTarget = target;
     }
 
     /**

diff --git a/packages/popover/test/basic.test.js b/packages/popover/test/basic.test.js
@@ -374,6 +374,75 @@ describe('popover', () => {
     });
   });
 
+  describe('detach and re-attach', () => {
+    let target;
+
+    beforeEach(() => {
+      target = fixtureSync('<button>Target</button>');
+    });
+
+    it('should not open on target click when detached', async () => {
+      popover.target = target;
+      await nextUpdate(popover);
+
+      popover.remove();
+      target.click();
+
+      expect(popover.opened).to.be.false;
+    });
+
+    it('should open on target click when re-attached', async () => {
+      popover.target = target;
+      await nextUpdate(popover);
+
+      popover.remove();
+
+      target.parentNode.appendChild(popover);
+      await nextUpdate(popover);
+
+      target.click();
+
+      expect(popover.opened).to.be.true;
+    });
+
+    it('should not open on target click when target set while detached', async () => {
+      popover.remove();
+
+      popover.target = target;
+      await nextUpdate(popover);
+
+      target.click();
+
+      expect(popover.opened).to.be.false;
+    });
+
+    it('should open when target set while detached after re-attached', async () => {
+      popover.remove();
+
+      popover.target = target;
+      await nextUpdate(popover);
+
+      target.parentNode.appendChild(popover);
+      await nextUpdate(popover);
+
+      target.click();
+
+      expect(popover.opened).to.be.true;
+    });
+
+    it('should not open on target click when target is cleared', async () => {
+      popover.target = target;
+      await nextUpdate(popover);
+
+      popover.target = null;
+      await nextUpdate(popover);
+
+      target.click();
+
+      expect(popover.opened).to.be.false;
+    });
+  });
+
   describe('dimensions', () => {
     function getStyleValue(element) {
       return element