fix(overlay): overlay closing another overlay (#4880)

adobe · Nov 11, 2024 · 30434fa · 30434fa
1 parent 48448ea
commit 30434fa
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 11 deletions.
diff --git a/packages/overlay/src/Overlay.ts b/packages/overlay/src/Overlay.ts
@@ -503,11 +503,12 @@ export class Overlay extends OverlayFeatures {
 
     protected handleBeforetoggle(event: Event & { newState: string }): void {
         if (event.newState !== 'open') {
-            this.handleBrowserClose();
+            this.handleBrowserClose(event);
         }
     }
 
-    protected handleBrowserClose(): void {
+    protected handleBrowserClose(event: Event): void {
+        event.stopPropagation();
         if (!this.strategy?.activelyOpening) {
             this.open = false;
             return;

diff --git a/packages/overlay/src/OverlayStack.ts b/packages/overlay/src/OverlayStack.ts
@@ -67,6 +67,8 @@ class OverlayStack {
         this.pointerdownPath = undefined;
         if (!this.stack.length) return;
         if (!composedPath?.length) return;
+        const lastOverlay = this.lastOverlay;
+        this.lastOverlay = undefined;
 
         const lastIndex = this.stack.length - 1;
         const nonAncestorOverlays = this.stack.filter((overlay, i) => {
@@ -80,13 +82,15 @@ class OverlayStack {
                     // The last Overlay in the stack is not the last Overlay at `pointerdown` time and has a
                     // `triggerInteraction` of "longpress", meaning it was opened by this poitner interaction
                     (i === lastIndex &&
-                        overlay !== this.lastOverlay &&
+                        overlay !== lastOverlay &&
                         overlay.triggerInteraction === 'longpress')
             );
             return (
                 !inStack &&
                 !overlay.shouldPreventClose() &&
-                overlay.type !== 'manual'
+                overlay.type !== 'manual' &&
+                // Don't close if this overlay is modal and not on top of the overlay stack.
+                !(overlay.type === 'modal' && lastOverlay !== overlay)
             );
         }) as Overlay[];
         nonAncestorOverlays.reverse();
@@ -140,8 +144,8 @@ class OverlayStack {
 
     /**
      * When overlays are added manage the open state of exisiting overlays appropriately:
-     * - 'modal': should close other overlays
-     * - 'page': should close other overlays
+     * - 'modal': should close other non-'modal' and non-'manual' overlays
+     * - 'page': should close other non-'modal' and non-'manual' overlays
      * - 'auto': should close other 'auto' overlays and other 'hint' overlays, but not 'manual' overlays
      * - 'manual': shouldn't close other overlays
      * - 'hint': shouldn't close other overlays and give way to all other overlays on a trigger
@@ -172,7 +176,11 @@ class OverlayStack {
                     const path = event.composedPath();
                     this.stack.forEach((overlayEl) => {
                         const inPath = path.find((el) => el === overlayEl);
-                        if (!inPath && overlayEl.type !== 'manual') {
+                        if (
+                            !inPath &&
+                            overlayEl.type !== 'manual' &&
+                            overlayEl.type !== 'modal'
+                        ) {
                             this.closeOverlay(overlayEl);
                         }
                     });

diff --git a/packages/overlay/test/overlay-element.test.ts b/packages/overlay/test/overlay-element.test.ts
@@ -389,7 +389,7 @@ describe('sp-overlay', () => {
                 this.page.open = false;
                 await closed;
             });
-            it('closes "page" overlays when opening', async function () {
+            it('should not close "modal" overlays when opening', async function () {
                 let opened = oneEvent(this.modal, 'sp-opened');
                 this.modal.open = true;
                 await opened;
@@ -400,11 +400,9 @@ describe('sp-overlay', () => {
                 expect(this.manual.open).to.be.false;
 
                 opened = oneEvent(this.page, 'sp-opened');
-                const closed = oneEvent(this.modal, 'sp-closed');
                 this.page.open = true;
                 await opened;
-                await closed;
-                expect(this.modal.open).to.be.false;
+                expect(this.modal.open).to.be.true;
                 expect(this.page.open).to.be.true;
                 expect(this.hint.open).to.be.false;
                 expect(this.auto.open).to.be.false;