refactor: reset overlay position when positionTarget is removed (#10107)

Author: web-padawan

packages/overlay/src/vaadin-overlay-position-mixin.js

@@ -116,13 +116,6 @@ export const PositionMixin = (superClass) =>
       };
     }
 
-    static get observers() {
-      return [
-        '__positionSettingsChanged(horizontalAlign, verticalAlign, noHorizontalOverlap, noVerticalOverlap, requiredVerticalSpace)',
-        '__overlayOpenedChanged(opened, positionTarget)',
-      ];
-    }
-
     constructor() {
       super();
 
@@ -145,6 +138,30 @@ export const PositionMixin = (superClass) =>
       this.__removeUpdatePositionEventListeners();
     }
 
+    /** @protected */
+    updated(props) {
+      super.updated(props);
+
+      if (props.has('opened') || props.has('positionTarget')) {
+        if (!this.positionTarget && props.get('positionTarget')) {
+          this.__resetPosition();
+        }
+
+        this.__overlayOpenedChanged(this.opened, this.positionTarget);
+      }
+
+      const positionProps = [
+        'horizontalAlign',
+        'verticalAlign',
+        'noHorizontalOverlap',
+        'noVerticalOverlap',
+        'requiredVerticalSpace',
+      ];
+      if (positionProps.some((prop) => props.has(prop))) {
+        this._updatePosition();
+      }
+    }
+
     /** @private */
     __addUpdatePositionEventListeners() {
       window.visualViewport.addEventListener('resize', this._updatePosition);
@@ -210,10 +227,6 @@ export const PositionMixin = (superClass) =>
       }
     }
 
-    __positionSettingsChanged() {
-      this._updatePosition();
-    }
-
     /** @private */
     __onScroll(e) {
       // If the scroll event occurred inside the overlay, ignore it.
@@ -224,6 +237,23 @@ export const PositionMixin = (superClass) =>
       this._updatePosition();
     }
 
+    /** @private */
+    __resetPosition() {
+      Object.assign(this.style, {
+        justifyContent: '',
+        alignItems: '',
+        top: '',
+        bottom: '',
+        left: '',
+        right: '',
+      });
+
+      setOverlayStateAttribute(this, 'bottom-aligned', false);
+      setOverlayStateAttribute(this, 'top-aligned', false);
+      setOverlayStateAttribute(this, 'end-aligned', false);
+      setOverlayStateAttribute(this, 'start-aligned', false);
+    }
+
     _updatePosition() {
       if (!this.positionTarget || !this.opened || !this.__margins) {
         return;

packages/overlay/test/position-mixin.test.js

@@ -106,6 +106,18 @@ describe('position mixin', () => {
     expect(overlay.opened).to.be.false;
   });
 
+  it('should reset styles when positionTarget is reset', async () => {
+    overlay.positionTarget = null;
+    await nextRender();
+
+    expect(overlay.style.top).to.equal('');
+    expect(overlay.style.bottom).to.equal('');
+    expect(overlay.style.left).to.equal('');
+    expect(overlay.style.right).to.equal('');
+    expect(overlay.style.alignItems).to.equal('');
+    expect(overlay.style.justifyContent).to.equal('');
+  });
+
   describe('vertical align top', () => {
     beforeEach(() => {
       overlay.verticalAlign = TOP;
@@ -125,6 +137,13 @@ describe('position mixin', () => {
       expect(parent.hasAttribute('bottom-aligned')).to.be.false;
     });
 
+    it('should remove top-aligned attribute when positionTarget is reset', async () => {
+      overlay.positionTarget = null;
+      await nextRender();
+      expect(overlay.hasAttribute('top-aligned')).to.be.false;
+      expect(parent.hasAttribute('top-aligned')).to.be.false;
+    });
+
     it('should align top edges when overlay part is animated', async () => {
       overlay.classList.add('animated');
       await oneEvent(overlay.$.overlay, 'animationend');
@@ -146,6 +165,16 @@ describe('position mixin', () => {
       expect(parent.hasAttribute('bottom-aligned')).to.be.true;
     });
 
+    it('should bottom top-aligned attribute when positionTarget is reset', async () => {
+      target.style.top = `${targetPositionToFlipOverlay + 3}px`;
+      updatePosition();
+
+      overlay.positionTarget = null;
+      await nextRender();
+      expect(overlay.hasAttribute('bottom-aligned')).to.be.false;
+      expect(parent.hasAttribute('bottom-aligned')).to.be.false;
+    });
+
     it('should flip when out of space and squeezed smaller than current available space', () => {
       target.style.top = `${targetPositionToFlipOverlay + 3}px`;
 
@@ -384,6 +413,13 @@ describe('position mixin', () => {
       expect(parent.hasAttribute('end-aligned')).to.be.false;
     });
 
+    it('should remove start-aligned attribute when positionTarget is reset', async () => {
+      overlay.positionTarget = null;
+      await nextRender();
+      expect(overlay.hasAttribute('start-aligned')).to.be.false;
+      expect(parent.hasAttribute('start-aligned')).to.be.false;
+    });
+
     it('should align right edges with right-to-left', async () => {
       document.dir = 'rtl';
       await nextRender();
@@ -407,6 +443,16 @@ describe('position mixin', () => {
       expect(parent.hasAttribute('end-aligned')).to.be.true;
     });
 
+    it('should remove end-aligned attribute when positionTarget is reset', async () => {
+      target.style.left = `${targetPositionToFlipOverlay + 3}px`;
+      updatePosition();
+
+      overlay.positionTarget = null;
+      await nextRender();
+      expect(overlay.hasAttribute('end-aligned')).to.be.false;
+      expect(parent.hasAttribute('end-aligned')).to.be.false;
+    });
+
     it('should flip when out of space and squeezed smaller than current available space', () => {
       target.style.left = `${targetPositionToFlipOverlay + 3}px`;