refactor: make context-menu opened property sync (#11650)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: vursen

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

@@ -36,6 +36,7 @@ export const ContextMenuMixin = (superClass) =>
           value: false,
           notify: true,
           readOnly: true,
+          sync: true,
         },
 
         /**
@@ -135,7 +136,7 @@ export const ContextMenuMixin = (superClass) =>
       this._boundOpen = this.open.bind(this);
       this._boundClose = this.close.bind(this);
       this._boundPreventDefault = this._preventDefault.bind(this);
-      this._boundOnGlobalContextMenu = this._onGlobalContextMenu.bind(this);
+      this.__onGlobalContextMenu = this.__onGlobalContextMenu.bind(this);
     }
 
     /** @protected */
@@ -144,6 +145,7 @@ export const ContextMenuMixin = (superClass) =>
 
       this.__boundOnScroll = this.__onScroll.bind(this);
       window.addEventListener('scroll', this.__boundOnScroll, true);
+      document.documentElement.addEventListener('contextmenu', this.__onGlobalContextMenu, true);
 
       // Restore opened state if overlay was opened when disconnecting
       if (this.__restoreOpened) {
@@ -156,10 +158,16 @@ export const ContextMenuMixin = (superClass) =>
       super.disconnectedCallback();
 
       window.removeEventListener('scroll', this.__boundOnScroll, true);
+      document.documentElement.removeEventListener('contextmenu', this.__onGlobalContextMenu, true);
 
-      // Close overlay and memorize opened state
+      // Memorize opened state and defer close so that DOM moves (disconnect
+      // followed by reconnect within the same task) do not close the overlay.
       this.__restoreOpened = this.opened;
-      this.close();
+      setTimeout(() => {
+        if (!this.isConnected) {
+          this.close();
+        }
+      });
     }
 
     /** @protected */
@@ -290,13 +298,7 @@ export const ContextMenuMixin = (superClass) =>
     }
 
     /** @private */
-    _openedChanged(opened, oldOpened) {
-      if (opened) {
-        document.documentElement.addEventListener('contextmenu', this._boundOnGlobalContextMenu, true);
-      } else if (oldOpened) {
-        document.documentElement.removeEventListener('contextmenu', this._boundOnGlobalContextMenu, true);
-      }
-
+    _openedChanged(opened) {
       this.__setListenOnUserSelect(opened);
     }
 
@@ -630,7 +632,10 @@ export const ContextMenuMixin = (superClass) =>
     }
 
     /** @private */
-    _onGlobalContextMenu(e) {
+    __onGlobalContextMenu(e) {
+      if (!this.opened) {
+        return;
+      }
       if (!e.shiftKey) {
         const isTouchDevice = isAndroid || isIOS;
         if (!isTouchDevice) {

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

@@ -374,9 +374,7 @@ export const ItemsMixin = (superClass) =>
 
       // Listen to the forwarded event from sub-menu.
       this.addEventListener('close-all-menus', () => {
-        // Call `close()` on the overlay to close synchronously,
-        // as we can't have `sync: true` on `opened` property.
-        this._overlayElement.close();
+        this.close();
       });
 
       // Listen to the forwarded event from sub-menu.

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

@@ -1,5 +1,5 @@
 import { expect } from '@vaadin/chai-plugins';
-import { fire, fixtureSync, nextRender } from '@vaadin/testing-helpers';
+import { aTimeout, fire, fixtureSync, nextRender } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import '../src/vaadin-context-menu.js';
 import '@vaadin/item/src/vaadin-item.js';
@@ -121,25 +121,27 @@ describe('context', () => {
     expect(menu.opened).to.be.true;
   });
 
-  it('should be closed after detached', () => {
+  it('should be closed after detached', async () => {
     fire(target, 'contextmenu');
     expect(menu.opened).to.be.true;
 
     const spy = sinon.spy(menu, 'close');
 
     menu.parentNode.removeChild(menu);
+    await aTimeout(0);
     expect(spy.calledOnce).to.be.true;
     expect(menu.opened).to.be.false;
   });
 
-  it('should not close when moved within the DOM', () => {
+  it('should not close when moved within the DOM', async () => {
     fire(target, 'contextmenu');
     expect(menu.opened).to.be.true;
 
     const newParent = document.createElement('div');
     document.body.appendChild(newParent);
 
     newParent.appendChild(menu);
+    await aTimeout(0);
     expect(menu.opened).to.be.true;
   });
 });

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

@@ -707,7 +707,7 @@ export const MenuBarMixin = (superClass) =>
 
       if (wasExpanded && button.item && button.item.children) {
         this.__openSubMenu(button, true, { keepFocus: true });
-      } else {
+      } else if (!this._subMenu.opened) {
         this._tooltipController.open({ trigger: 'focus' });
       }
     }
@@ -761,7 +761,7 @@ export const MenuBarMixin = (superClass) =>
           this._setTabindex(btn, btn === target);
         });
 
-        if (isKeyboardActive()) {
+        if (!this._subMenu.opened && isKeyboardActive()) {
           this._tooltipController.open({ trigger: 'focus' });
         }
       } else {

packages/menu-bar/src/vaadin-menu-bar-submenu.js

@@ -102,13 +102,21 @@ class MenuBarSubmenu extends ContextMenuMixin(ThemePropertyMixin(PolylitMixin(Li
   }
 
   /**
-   * Overriding the observer to not add global "contextmenu" listener.
+   * Overriding the observer to not toggle user-select on the host.
    * @override
    */
   _openedChanged() {
     // Do nothing
   }
 
+  /**
+   * Overriding the handler to not react to global "contextmenu" events.
+   * @override
+   */
+  __onGlobalContextMenu() {
+    // Do nothing
+  }
+
   /**
    * Overriding the public method to reset expanded button state.
    */