refactor: update context-menu items when calling requestContentUpdate (#9331)

* refactor: update context-menu items when calling requestContentUpdate

* fix: do not request update for submenu if it was closed, add test

Author: web-padawan

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

@@ -3,7 +3,7 @@
  * Copyright (c) 2016 - 2025 Vaadin Ltd.
  * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
  */
-import { isElementFocusable } from '@vaadin/a11y-base/src/focus-utils.js';
+import { isElementFocusable, isKeyboardActive } from '@vaadin/a11y-base/src/focus-utils.js';
 import { isAndroid, isIOS } from '@vaadin/component-base/src/browser-utils.js';
 import { addListener, deepTargetFind, gestures, removeListener } from '@vaadin/component-base/src/gestures.js';
 import { MediaQueryController } from '@vaadin/component-base/src/media-query-controller.js';
@@ -338,11 +338,18 @@ export const ContextMenuMixin = (superClass) =>
      * It is not guaranteed that the update happens immediately (synchronously) after it is requested.
      */
     requestContentUpdate() {
-      if (!this._overlayElement || !this.renderer) {
+      if (!this._overlayElement) {
         return;
       }
 
+      // Store state to be restored after re-rendering
+      this.__preserveMenuState();
+
+      // Run overlay renderer to re-create DOM elements
       this._overlayElement.requestContentUpdate();
+
+      // Restore focused item, update sub-menu if needed
+      this.__restoreMenuState();
     }
 
     /** @private */
@@ -410,6 +417,69 @@ export const ContextMenuMixin = (superClass) =>
       }
     }
 
+    /** @private */
+    __preserveMenuState() {
+      const listBox = this.__getListBox();
+      if (listBox) {
+        this.__focusedIndex = listBox.items.indexOf(listBox.focused);
+
+        if (this._subMenu && this._subMenu.opened) {
+          this.__subMenuIndex = listBox.items.indexOf(this._subMenu.listenOn);
+        }
+      }
+    }
+
+    /** @private */
+    __restoreMenuState() {
+      const focusedIndex = this.__focusedIndex;
+      const subMenuIndex = this.__subMenuIndex;
+      const selectedIndex = this.__selectedIndex;
+
+      const listBox = this.__getListBox();
+
+      if (listBox) {
+        // Initialize menu items synchronously
+        listBox._observer.flush();
+
+        if (subMenuIndex > -1) {
+          const itemToOpen = listBox.items[subMenuIndex];
+          if (itemToOpen) {
+            if (Array.isArray(itemToOpen._item.children) && itemToOpen._item.children.length) {
+              // Keep nested sub-menu opened and update it
+              this.__updateSubMenuForItem(this._subMenu, itemToOpen);
+              this._subMenu.requestContentUpdate();
+            } else {
+              // Item no longer has children, close sub-menu
+              this._subMenu.close();
+              this.__focusItem(itemToOpen);
+            }
+          } else {
+            // Item no longer exists, focus the list-box
+            listBox.focus();
+          }
+        }
+
+        // Focus previously selected item, in case it was clicked with
+        // `keepOpen` option, or the previously focused item, if any
+        this.__focusItem(selectedIndex > -1 ? listBox.children[selectedIndex] : listBox.items[focusedIndex]);
+      }
+
+      this.__focusedIndex = undefined;
+      this.__subMenuIndex = undefined;
+      this.__selectedIndex = undefined;
+    }
+
+    /** @private */
+    __focusItem(item) {
+      if (item) {
+        item.focus();
+
+        if (isKeyboardActive()) {
+          item.setAttribute('focus-ring', '');
+        }
+      }
+    }
+
     /** @private */
     __onScroll() {
       if (!this.opened) {

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

@@ -132,14 +132,12 @@ export const ItemsMixin = (superClass) =>
 
     /** @private */
     __openSubMenu(subMenu, itemElement, overlayClass) {
-      subMenu.items = itemElement._item.children;
-      subMenu.listenOn = itemElement;
+      this.__updateSubMenuForItem(subMenu, itemElement);
       subMenu.overlayClass = overlayClass;
 
       const parent = this._overlayElement;
 
       const subMenuOverlay = subMenu._overlayElement;
-      subMenuOverlay.positionTarget = itemElement;
       subMenuOverlay.noHorizontalOverlap = true;
       // Store the reference parent overlay
       subMenuOverlay._setParentOverlay(parent);
@@ -163,6 +161,13 @@ export const ItemsMixin = (superClass) =>
       );
     }
 
+    /** @private */
+    __updateSubMenuForItem(subMenu, itemElement) {
+      subMenu.items = itemElement._item.children;
+      subMenu.listenOn = itemElement;
+      subMenu._overlayElement.positionTarget = itemElement;
+    }
+
     /**
      * @param {!ContextMenuItem} item
      * @return {HTMLElement}
@@ -317,14 +322,10 @@ export const ItemsMixin = (superClass) =>
         const menu = e.target;
         const selectedItem = e.detail.value;
         const index = menu.items.indexOf(selectedItem);
-        if (!!selectedItem.keepOpen && index > -1) {
-          menu._overlayElement.requestContentUpdate();
-
-          // Initialize items synchronously
-          menu._listBox._observer.flush();
-
-          const newItem = menu._listBox.children[index];
-          newItem.focus();
+        // Menu can be no longer opened if parent menu items changed
+        if (!!selectedItem.keepOpen && index > -1 && menu.opened) {
+          menu.__selectedIndex = index;
+          menu.requestContentUpdate();
         } else if (!selectedItem.keepOpen) {
           this.close();
         }
@@ -391,22 +392,27 @@ export const ItemsMixin = (superClass) =>
       }
     }
 
+    /** @protected */
+    __getListBox() {
+      return this._overlayElement.querySelector(`${this._tagNamePrefix}-list-box`);
+    }
+
     /**
      * @param {!HTMLElement} root
      * @param {!ContextMenu} menu
      * @param {!ContextMenuRendererContext} context
      * @protected
      */
-    __itemsRenderer(root, menu, { detail }) {
+    __itemsRenderer(root, menu) {
       this.__initMenu(root, menu);
 
       const subMenu = root.querySelector(this.constructor.is);
       subMenu.closeOn = menu.closeOn;
 
-      const listBox = root.querySelector(`${this._tagNamePrefix}-list-box`);
+      const listBox = this.__getListBox();
       listBox.innerHTML = '';
 
-      [...(detail.children || menu.items)].forEach((item) => {
+      menu.items.forEach((item) => {
         const component = this.__createComponent(item);
         listBox.appendChild(component);
       });

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

@@ -43,7 +43,7 @@ describe('items', () => {
         children: [
           { text: 'foo-0-0', checked: true },
           { text: 'foo-0-1', disabled: true },
-          { text: 'foo-0-2', children: [{ text: 'foo-0-2-0' }] },
+          { text: 'foo-0-2', children: [{ text: 'foo-0-2-0', keepOpen: true }] },
           { component: 'hr' },
           { text: 'foo-0-3', keepOpen: true },
         ],
@@ -493,10 +493,10 @@ describe('items', () => {
     }).to.throw(Error);
   });
 
-  it('should not call requestContentUpdate', () => {
+  it('should call requestContentUpdate on the overlay', () => {
     const spy = sinon.spy(rootOverlay, 'requestContentUpdate');
     rootMenu.requestContentUpdate();
-    expect(spy.called).to.be.false;
+    expect(spy.called).to.be.true;
   });
 
   it('should not remove the component attributes', async () => {
@@ -586,4 +586,99 @@ describe('items', () => {
       expect(subOverlay2.getBoundingClientRect().left).to.be.closeTo(subBRCLeft2 - scrollDistance, 1);
     });
   });
+
+  describe('updating while opened', () => {
+    it('should update items when calling requestContentUpdate while opened', async () => {
+      rootMenu.items = [{ text: 'foo-1' }, { text: 'foo-2' }];
+      rootMenu.requestContentUpdate();
+      await nextRender();
+      const items = getMenuItems(rootMenu);
+      expect(items.length).to.equal(2);
+      expect(items[0].textContent).to.equal('foo-1');
+      expect(items[1].textContent).to.equal('foo-2');
+    });
+
+    it('should keep submenu opened and update its items after calling requestContentUpdate while opened', async () => {
+      rootMenu.items = [
+        {
+          text: 'foo-1',
+          children: [{ text: 'foo-1-1' }, { text: 'foo-1-2' }],
+        },
+        { text: 'foo-2' },
+      ];
+      rootMenu.requestContentUpdate();
+      await nextRender();
+      expect(subMenu.opened).to.be.true;
+      const items = getMenuItems(subMenu);
+      expect(items[0].textContent).to.equal('foo-1-1');
+      expect(items[1].textContent).to.equal('foo-1-2');
+    });
+
+    it('should close submenu and focus parent item after calling requestContentUpdate while opened', async () => {
+      rootMenu.items = [{ text: 'foo-1' }, { text: 'foo-2' }];
+      rootMenu.requestContentUpdate();
+      await nextRender();
+      expect(subMenu.opened).to.be.false;
+      expect(getMenuItems(rootMenu)[0].hasAttribute('focused')).to.be.true;
+    });
+
+    it('should close submenu from item-selected event and focus parent item after calling requestContentUpdate while opened', async () => {
+      // Open nested sub-menu
+      await openMenu(getMenuItems(subMenu)[2]);
+
+      rootMenu.addEventListener('item-selected', () => {
+        rootMenu.items = [
+          {
+            text: 'foo-0',
+            children: [
+              { text: 'foo-0-0', checked: true },
+              { text: 'foo-0-1', disabled: true },
+              { text: 'foo-0-2', children: [] },
+            ],
+          },
+        ];
+        rootMenu.requestContentUpdate();
+      });
+
+      const subMenu2 = getSubMenu(subMenu);
+      const item = getMenuItems(subMenu2)[0];
+      item.focus();
+      enterKeyDown(item);
+
+      await nextRender();
+      expect(subMenu2.opened).to.be.false;
+      expect(subMenu.opened).to.be.true;
+      expect(getMenuItems(subMenu)[2].hasAttribute('focused')).to.be.true;
+    });
+
+    it('should close submenu and focus first available item after calling requestContentUpdate while opened', async () => {
+      subMenu.close();
+      await nextRender();
+
+      // Open sub-menu for a last item
+      const parent = getMenuItems(rootMenu)[3];
+      await openMenu(parent);
+
+      rootMenu.items = [{ text: 'foo-1' }, { text: 'foo-2' }];
+      rootMenu.requestContentUpdate();
+      await nextRender();
+
+      expect(subMenu.opened).to.be.false;
+      expect(getMenuItems(rootMenu)[0].hasAttribute('focused')).to.be.true;
+    });
+
+    it('should focus previously focused item after calling requestContentUpdate while opened', async () => {
+      subMenu.close();
+      await nextRender();
+
+      // Focus an item without a submenu
+      getMenuItems(rootMenu)[1].focus();
+
+      rootMenu.items = [{ text: 'foo-1' }, { text: 'foo-2' }];
+      rootMenu.requestContentUpdate();
+      await nextRender();
+
+      expect(getMenuItems(rootMenu)[1].hasAttribute('focused')).to.be.true;
+    });
+  });
 });