fix: prevent leaving multiple submenus open at a time

adobe · Jun 3, 2022 · d2bfbb2 · d2bfbb2
1 parent e3a08b4
commit d2bfbb2
Show file tree

Hide file tree

Showing 8 changed files with 366 additions and 118 deletions.
diff --git a/packages/menu/src/Menu.ts b/packages/menu/src/Menu.ts
@@ -47,11 +47,6 @@ function elementIsOrContains(
     return !!isOrContains && (el === isOrContains || el.contains(isOrContains));
 }
 
-/* c8 ignore next 3 */
-const noop = (): void => {
-    return;
-};
-
 /**
  * Spectrum Menu Component
  * @element sp-menu
@@ -70,44 +65,8 @@ export class Menu extends SpectrumElement {
         return [menuStyles];
     }
 
-    public get closeSelfAsSubmenu(): (leave?: boolean) => void {
-        this.isSubmenu = false;
-        return this._closeSelfAsSubmenu;
-    }
-
-    public setCloseSelfAsSubmenu(cb: (leave?: boolean) => void): void {
-        if (cb === this._closeSelfAsSubmenu) {
-            this.isSubmenu = false;
-            this._closeSelfAsSubmenu = noop;
-            return;
-        }
-        this.isSubmenu = true;
-        this._closeSelfAsSubmenu = cb;
-    }
-
-    _closeSelfAsSubmenu = noop;
-
     public isSubmenu = false;
 
-    public get closeOpenSubmenu(): (leave?: boolean) => void {
-        this.hasOpenSubmenu = false;
-        return this._closeOpenSubmenu;
-    }
-
-    public setCloseOpenSubmenu(cb: (leave?: boolean) => void): void {
-        if (cb === this._closeOpenSubmenu) {
-            this.hasOpenSubmenu = false;
-            this._closeOpenSubmenu = noop;
-            return;
-        }
-        this.hasOpenSubmenu = true;
-        this._closeOpenSubmenu = cb;
-    }
-
-    _closeOpenSubmenu = noop;
-
-    public hasOpenSubmenu = false;
-
     @property({ type: String, reflect: true })
     public label = '';
 
@@ -315,11 +274,6 @@ export class Menu extends SpectrumElement {
         }
     }
 
-    public submenuWillCloseOn(menuItem: MenuItem): void {
-        this.focusedItemIndex = this.childItems.indexOf(menuItem);
-        this.focusInItemIndex = this.focusedItemIndex;
-    }
-
     private onClick(event: Event): void {
         if (event.defaultPrevented) {
             return;
@@ -342,12 +296,6 @@ export class Menu extends SpectrumElement {
             if (target.hasSubmenu || target.open) {
                 return;
             }
-            if (this.hasOpenSubmenu) {
-                this.closeOpenSubmenu();
-            }
-            if (this.isSubmenu) {
-                this.closeSelfAsSubmenu();
-            }
             this.selectOrToggleItem(target);
         } else {
             return;
@@ -514,10 +462,10 @@ export class Menu extends SpectrumElement {
                 // Remove focus while opening overlay from keyboard or the visible focus
                 // will slip back to the first item in the menu.
                 this.blur();
-                lastFocusedItem.openOverlay({ immediate: true });
+                lastFocusedItem.openOverlay();
             }
-        } else if (this.isSubmenu && shouldCloseSelfAsSubmenu) {
-            this.closeSelfAsSubmenu(true);
+        } else if (shouldCloseSelfAsSubmenu && this.isSubmenu) {
+            this.dispatchEvent(new Event('close', { bubbles: true }));
         }
     }
 
@@ -533,7 +481,7 @@ export class Menu extends SpectrumElement {
                 // Remove focus while opening overlay from keyboard or the visible focus
                 // will slip back to the first item in the menu.
                 this.blur();
-                lastFocusedItem.openOverlay({ immediate: true });
+                lastFocusedItem.openOverlay();
                 return;
             }
         }

diff --git a/packages/menu/src/MenuItem.ts b/packages/menu/src/MenuItem.ts
@@ -327,7 +327,7 @@ export class MenuItem extends LikeAnchor(Focusable) {
     public closeOverlay?: (leave?: boolean) => Promise<void>;
 
     protected handleSubmenuClick(): void {
-        this.openOverlay({ immediate: true });
+        this.openOverlay();
     }
 
     protected handlePointerenter(): void {
@@ -368,9 +368,7 @@ export class MenuItem extends LikeAnchor(Focusable) {
         }
     };
 
-    public async openOverlay({
-        immediate,
-    }: { immediate?: boolean } = {}): Promise<void> {
+    public async openOverlay(): Promise<void> {
         if (!this.hasSubmenu || this.open || this.disabled) {
             return;
         }
@@ -393,60 +391,34 @@ export class MenuItem extends LikeAnchor(Focusable) {
                 const slotName = el.slot;
                 el.tabIndex = 0;
                 el.removeAttribute('slot');
+                el.isSubmenu = true;
                 return (el) => {
                     el.tabIndex = -1;
                     el.slot = slotName;
+                    el.isSubmenu = false;
                 };
             },
         });
         const closeOverlay = openOverlay(this, 'click', popover, {
             placement: this.isLTR ? 'right-start' : 'left-start',
             receivesFocus: 'auto',
-            delayed: !immediate && false,
+            root: this.menuData.focusRoot,
         });
-        let closing = false;
-        const closeSubmenu = async (leave = false): Promise<void> => {
+        const closeSubmenu = async (): Promise<void> => {
             delete this.closeOverlay;
-            if (submenu.hasOpenSubmenu) {
-                await submenu.closeOpenSubmenu(leave);
-            }
-            if (!leave) {
-                closing = true;
-            }
-            this.menuData.focusRoot?.submenuWillCloseOn(this);
             (await closeOverlay)();
         };
         this.closeOverlay = closeSubmenu;
-        if (this.menuData.focusRoot?.hasOpenSubmenu) {
-            this.menuData.focusRoot.closeOpenSubmenu(true);
-        }
-        const setup = (): void => {
-            submenu.setCloseSelfAsSubmenu(closeSubmenu);
-            this.menuData.focusRoot?.setCloseOpenSubmenu(closeSubmenu);
-        };
         const cleanup = (event: CustomEvent<OverlayOpenCloseDetail>): void => {
             event.stopPropagation();
             returnSubmenu();
-            submenu.setCloseSelfAsSubmenu(closeSubmenu);
-            this.menuData.focusRoot?.setCloseOpenSubmenu(closeSubmenu);
             this.open = false;
             this.active = false;
-            if (closing || event.detail.reason === 'external-click') {
-                this.menuData.focusRoot?.dispatchEvent(
-                    new CustomEvent('close', {
-                        bubbles: true,
-                        composed: true,
-                        detail: { reason: 'external-click' },
-                    })
-                );
-            }
         };
-        this.addEventListener('sp-opened', setup as EventListener, {
-            once: true,
-        });
         this.addEventListener('sp-closed', cleanup as EventListener, {
             once: true,
         });
+        popover.addEventListener('change', closeSubmenu);
     }
 
     updateAriaSelected(): void {