fix: prevent closing submenu when switching between items (#10152)

Author: sissbruecker

packages/context-menu/src/vaadin-contextmenu-items-mixin.js

@@ -138,10 +138,10 @@ export const ItemsMixin = (superClass) =>
 
     /** @private */
     __openSubMenu(subMenu, itemElement) {
+      // Update sub-menu items and position target
       this.__updateSubMenuForItem(subMenu, itemElement);
 
       const parent = this._overlayElement;
-
       const subMenuOverlay = subMenu._overlayElement;
       // Store the reference parent overlay
       subMenuOverlay._setParentOverlay(parent);
@@ -170,6 +170,7 @@ export const ItemsMixin = (superClass) =>
       subMenu.items = itemElement._item.children;
       subMenu.listenOn = itemElement;
       subMenu._positionTarget = itemElement;
+      subMenu._overlayElement.requestContentUpdate();
     }
 
     /**
@@ -371,24 +372,32 @@ export const ItemsMixin = (superClass) =>
       }
 
       const subMenu = this._subMenu;
+      const expandedItem = this._listBox.querySelector('[expanded]');
 
-      if (item) {
+      if (item && item !== expandedItem) {
         const { children } = item._item;
 
         // Check if the sub-menu was focused before closing it.
         const child = subMenu._overlayElement._contentRoot.firstElementChild;
         const isSubmenuFocused = child && child.focused;
 
-        if (subMenu.items !== children) {
+        // Mark previously expanded item as collapsed
+        if (expandedItem) {
+          this.__updateExpanded(expandedItem, false);
+        }
+
+        // Close sub-menu if there are no children for the new item
+        if (!children || !children.length) {
           subMenu.close();
         }
+
         if (!this.opened) {
           return;
         }
 
         if (children && children.length) {
+          // Open or update the submenu if the new item has children
           this.__updateExpanded(item, true);
-
           this.__openSubMenu(subMenu, item);
         } else if (isSubmenuFocused) {
           // If the sub-menu item was focused, focus its parent item.

packages/context-menu/test/helpers.js

@@ -19,8 +19,15 @@ export async function openMenu(target, event = isTouch ? 'click' : 'mouseover')
   }
   // Disable logic that delays opening submenu
   menu.__openListenerActive = true;
+
+  // Open the submenu
+  const wasOpened = menu._overlayElement.opened;
   activateItem(target, event);
-  await oneEvent(menu._overlayElement, 'vaadin-overlay-open');
+
+  // Wait for the submenu to open, unless it was already opened for a different item
+  if (!wasOpened) {
+    await oneEvent(menu._overlayElement, 'vaadin-overlay-open');
+  }
 }
 
 export function getMenuItems(menu) {

packages/context-menu/test/items.test.js

@@ -242,6 +242,7 @@ describe('items', () => {
   it('should not have a checked item', async () => {
     rootMenu.items[0].children[0].checked = false;
     subMenu.close();
+    await nextRender();
     await openMenu(getMenuItems(rootMenu)[0]);
     expect(getMenuItems(subMenu)[0].hasAttribute('menu-item-checked')).to.be.false;
   });
@@ -250,6 +251,27 @@ describe('items', () => {
     expect(getMenuItems(subMenu)[1].disabled).to.be.true;
   });
 
+  it('should update the submenu when activating other parent item', () => {
+    activateItem(getMenuItems(rootMenu)[3]);
+
+    expect(subMenu.opened).to.be.true;
+
+    const items = getMenuItems(subMenu);
+    expect(items.length).to.equal(3);
+    expect(items[0].textContent).to.equal('foo-3-0');
+    expect(items[1].textContent).to.equal('foo-3-1');
+    expect(items[2].textContent).to.equal('foo-3-2');
+  });
+
+  it('should not change opened state of the submenu when activating other parent item', () => {
+    const openedChangeSpy = sinon.spy();
+    subMenu.addEventListener('opened-changed', openedChangeSpy);
+
+    activateItem(getMenuItems(rootMenu)[3]);
+
+    expect(openedChangeSpy.called).to.be.false;
+  });
+
   it('should close the submenu on activating non-parent item', () => {
     activateItem(getMenuItems(rootMenu)[1]);
     expect(subMenu.opened).to.be.false;
@@ -349,8 +371,9 @@ describe('items', () => {
     expect(getMenuItems(rootMenu)[0].getAttribute('aria-haspopup')).to.equal('false');
   });
 
-  it('should open item on right arrow', () => {
+  it('should open item on right arrow', async () => {
     subMenu.close();
+    await nextRender();
     arrowRightKeyDown(getMenuItems(rootMenu)[0]);
     expect(subMenu.opened).to.be.true;
   });
@@ -359,25 +382,29 @@ describe('items', () => {
     document.documentElement.setAttribute('dir', 'rtl');
     await nextFrame();
     subMenu.close();
+    await nextRender();
     arrowLeftKeyDown(getMenuItems(rootMenu)[0]);
     expect(subMenu.opened).to.be.true;
     document.documentElement.setAttribute('dir', 'ltr');
   });
 
-  it('should open item on enter', () => {
+  it('should open item on enter', async () => {
     subMenu.close();
+    await nextRender();
     enterKeyDown(getMenuItems(rootMenu)[0]);
     expect(subMenu.opened).to.be.true;
   });
 
-  it('should open item on space', () => {
+  it('should open item on space', async () => {
     subMenu.close();
+    await nextRender();
     spaceKeyDown(getMenuItems(rootMenu)[0]);
     expect(subMenu.opened).to.be.true;
   });
 
   it('should not focus item if parent item is not focused', async () => {
     subMenu.close();
+    await nextRender();
     rootOverlay.focus();
     await openMenu(getMenuItems(rootMenu)[0]);
     expect(subMenu.opened).to.be.true;
@@ -387,6 +414,7 @@ describe('items', () => {
 
   it('should focus first item in submenu on overlay element arrow down', async () => {
     subMenu.close();
+    await nextRender();
     rootOverlay.focus();
     await openMenu(getMenuItems(rootMenu)[0]);
     const item = getMenuItems(subMenu)[0];
@@ -397,6 +425,7 @@ describe('items', () => {
 
   it('should focus last item in submenu on overlay element arrow up', async () => {
     subMenu.close();
+    await nextRender();
     rootOverlay.focus();
     await openMenu(getMenuItems(rootMenu)[0]);
     const items = getMenuItems(subMenu);
@@ -408,6 +437,7 @@ describe('items', () => {
 
   it('should focus first item after re-opening when using components', async () => {
     subMenu.close();
+    await nextRender();
     rootOverlay.focus();
 
     const rootItem = getMenuItems(rootMenu)[3];
@@ -434,6 +464,7 @@ describe('items', () => {
 
   it('should focus first non-disabled item after re-opening when using components', async () => {
     subMenu.close();
+    await nextRender();
     rootOverlay.focus();
 
     rootMenu.items[3].children[0].disabled = true;
@@ -529,6 +560,23 @@ describe('items', () => {
     expect(getMenuItems(rootMenu)[0].getAttribute('aria-expanded')).to.equal('false');
   });
 
+  it('should update expanded attributes when activating different parent items', async () => {
+    expect(getMenuItems(rootMenu)[0].hasAttribute('expanded')).to.be.true;
+    expect(getMenuItems(rootMenu)[0].getAttribute('aria-expanded')).to.equal('true');
+
+    await activateItem(getMenuItems(rootMenu)[3]);
+    expect(getMenuItems(rootMenu)[0].hasAttribute('expanded')).to.be.false;
+    expect(getMenuItems(rootMenu)[0].getAttribute('aria-expanded')).to.equal('false');
+    expect(getMenuItems(rootMenu)[3].hasAttribute('expanded')).to.be.true;
+    expect(getMenuItems(rootMenu)[3].getAttribute('aria-expanded')).to.equal('true');
+
+    await activateItem(getMenuItems(rootMenu)[0]);
+    expect(getMenuItems(rootMenu)[0].hasAttribute('expanded')).to.be.true;
+    expect(getMenuItems(rootMenu)[0].getAttribute('aria-expanded')).to.equal('true');
+    expect(getMenuItems(rootMenu)[3].hasAttribute('expanded')).to.be.false;
+    expect(getMenuItems(rootMenu)[3].getAttribute('aria-expanded')).to.equal('false');
+  });
+
   (isTouch ? describe.skip : describe)('scrolling', () => {
     let rootOverlay, subOverlay1, subOverlay2, scrollElm;