fix(overlay): ensure overlay addition occurs after closing previous

adobe · Aug 12, 2020 · 7d2b102 · 7d2b102
1 parent 697e19e
commit 7d2b102
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 33 deletions.
diff --git a/packages/dropdown/test/dropdown.test.ts b/packages/dropdown/test/dropdown.test.ts
@@ -124,6 +124,47 @@ describe('Dropdown', () => {
 
         expect(el.open).to.be.false;
     });
+    it('closes when clicking away', async () => {
+        const el = await dropdownFixture();
+        el.id = 'closing';
+        const other = document.createElement('div');
+        document.body.append(other);
+
+        await elementUpdated(el);
+
+        expect(el.open).to.be.false;
+        el.click();
+        await elementUpdated(el);
+
+        expect(el.open).to.be.true;
+        other.click();
+        await waitUntil(() => !el.open, 'closed');
+
+        other.remove();
+    });
+    it('toggles between dropdowns', async () => {
+        const el2 = await dropdownFixture();
+        const el1 = await dropdownFixture();
+
+        el1.id = 'away';
+        el2.id = 'other';
+
+        await Promise.all([elementUpdated(el1), elementUpdated(el2)]);
+
+        expect(el1.open, 'closed 1').to.be.false;
+        expect(el2.open, 'closed 1').to.be.false;
+        el1.click();
+        await Promise.all([elementUpdated(el1), elementUpdated(el2)]);
+        await waitUntil(() => el1.open && !el2.open, '1 open, 2 closed');
+
+        el2.click();
+        await Promise.all([elementUpdated(el1), elementUpdated(el2)]);
+        await waitUntil(() => !el1.open && el2.open, '1 closed, 2 open');
+
+        el1.click();
+        await Promise.all([elementUpdated(el1), elementUpdated(el2)]);
+        await waitUntil(() => el1.open && !el2.open, '1 open, 2 closed: again');
+    });
     it('selects', async () => {
         const el = await dropdownFixture();
 

diff --git a/packages/overlay/src/ActiveOverlay.ts b/packages/overlay/src/ActiveOverlay.ts
@@ -184,6 +184,13 @@ export class ActiveOverlay extends LitElement {
         return [styles];
     }
 
+    public constructor() {
+        super();
+        this.stealOverlayContentPromise = new Promise(
+            (res) => (this.stealOverlayContentResolver = res)
+        );
+    }
+
     public feature(): void {
         this.tabIndex = 0;
         if (this.interaction === 'modal') {
@@ -345,6 +352,8 @@ export class ActiveOverlay extends LitElement {
         this.originalPlacement = this.overlayContent.getAttribute(
             'placement'
         ) as Placement;
+
+        this.stealOverlayContentResolver();
     }
 
     private returnOverlayContent(): void {
@@ -488,4 +497,12 @@ export class ActiveOverlay extends LitElement {
 
         return overlay;
     }
+
+    private stealOverlayContentPromise = Promise.resolve();
+    private stealOverlayContentResolver!: () => void;
+
+    protected async _getUpdateComplete(): Promise<void> {
+        await super._getUpdateComplete();
+        await this.stealOverlayContentPromise;
+    }
 }
diff --git a/packages/overlay/src/overlay-stack.ts b/packages/overlay/src/overlay-stack.ts
@@ -173,19 +173,22 @@ export class OverlayStack {
         }
 
         const activeOverlay = ActiveOverlay.create(details);
-        this.overlays.push(activeOverlay);
         document.body.appendChild(activeOverlay);
-        let updateComplete = await activeOverlay.updateComplete;
-        while (updateComplete === false) {
-            updateComplete = await activeOverlay.updateComplete;
-        }
-
-        this.addOverlayEventListeners(activeOverlay);
-        if (details.receivesFocus === 'auto') {
-            activeOverlay.focus();
-        }
 
-        return false;
+        /**
+         * The following work to make the new overlay the "top" of the stack
+         * has to happen AFTER the current call stack completes in case there
+         * is work there in to remove the previous "top" overlay.
+         */
+        return Promise.resolve().then(async () => {
+            this.overlays.push(activeOverlay);
+            await activeOverlay.updateComplete;
+            this.addOverlayEventListeners(activeOverlay);
+            if (details.receivesFocus === 'auto') {
+                activeOverlay.focus();
+            }
+            return false;
+        });
     }
 
     public addOverlayEventListeners(activeOverlay: ActiveOverlay): void {

diff --git a/packages/overlay/test/overlay-trigger.test.ts b/packages/overlay/test/overlay-trigger.test.ts
@@ -433,16 +433,16 @@ describe('Overlay Trigger', () => {
 
         outerButton.click();
 
-        // Wait for the DOM node to be stolen and reparented into the overlay
-        await waitForPredicate(
-            () => !(outerPopover.parentElement instanceof OverlayTrigger)
+        await waitUntil(
+            () => !(outerPopover.parentElement instanceof OverlayTrigger),
+            'outer content stolen and reparented'
         );
 
         innerButton.click();
 
-        // Wait for the DOM node to be stolen and reparented into the overlay
-        await waitForPredicate(
-            () => !(innerPopover.parentElement instanceof OverlayTrigger)
+        await waitUntil(
+            () => !(innerPopover.parentElement instanceof OverlayTrigger),
+            'inner content stolen and reparented'
         );
 
         expect(isVisible(outerPopover)).to.be.true;
@@ -455,19 +455,19 @@ describe('Overlay Trigger', () => {
 
         pressEscape();
 
-        // Wait for the DOM node to be put back in its original place
-        await waitForPredicate(
-            () => innerPopover.parentElement instanceof OverlayTrigger
+        await waitUntil(
+            () => innerPopover.parentElement instanceof OverlayTrigger,
+            'inner content returned'
         );
 
         expect(isVisible(outerPopover)).to.be.true;
         expect(isVisible(innerPopover)).to.be.false;
 
         pressEscape();
 
-        // Wait for the DOM node to be put back in its original place
-        await waitForPredicate(
-            () => outerPopover.parentElement instanceof OverlayTrigger
+        await waitUntil(
+            () => outerPopover.parentElement instanceof OverlayTrigger,
+            'outer content returned'
         );
 
         expect(isVisible(outerPopover)).to.be.false;
@@ -486,16 +486,16 @@ describe('Overlay Trigger', () => {
 
         outerButton.click();
 
-        // Wait for the DOM node to be stolen and reparented into the overlay
-        await waitForPredicate(
-            () => !(outerPopover.parentElement instanceof OverlayTrigger)
+        await waitUntil(
+            () => !(outerPopover.parentElement instanceof OverlayTrigger),
+            'outer content stolen and reparented'
         );
 
         innerButton.click();
 
-        // Wait for the DOM node to be stolen and reparented into the overlay
-        await waitForPredicate(
-            () => !(innerPopover.parentElement instanceof OverlayTrigger)
+        await waitUntil(
+            () => !(innerPopover.parentElement instanceof OverlayTrigger),
+            'inner content stolen and reparented'
         );
 
         expect(isVisible(outerPopover)).to.be.true;
@@ -512,8 +512,9 @@ describe('Overlay Trigger', () => {
         document.body.click();
 
         // Wait for the DOM node to be put back in its original place
-        await waitForPredicate(
-            () => innerPopover.parentElement instanceof OverlayTrigger
+        await waitUntil(
+            () => innerPopover.parentElement instanceof OverlayTrigger,
+            'outer content returned'
         );
 
         expect(isVisible(outerPopover)).to.be.true;
@@ -522,8 +523,9 @@ describe('Overlay Trigger', () => {
         document.body.click();
 
         // Wait for the DOM node to be put back in its original place
-        await waitForPredicate(
-            () => outerPopover.parentElement instanceof OverlayTrigger
+        await waitUntil(
+            () => outerPopover.parentElement instanceof OverlayTrigger,
+            'inner content returned'
         );
 
         expect(isVisible(outerPopover)).to.be.false;