fix(menu-button): close behavior and ensure menu overlays (#21184)

* fix(menu-button): parity close behavior, ensure menu overlays

* fix(menu-button): close menu on item activation and blur

* test: add tests

Author: heloiselui

packages/web-components/src/components/menu-button/__tests__/__snapshots__/menu-button-test.snap.js

@@ -6,12 +6,13 @@ snapshots['cds-menu-button should render and match snapshot'] =
   kind="primary"
   label="Actions"
   menu-alignment="bottom"
+  menu-background-token="layer"
   size="lg"
   tab-index="0"
 >
   <cds-menu
     size="lg"
-    style="inset-inline-start: 8px; inset-inline-end: initial; inset-block-start: 8px;"
+    style=""
   >
     <cds-menu-item
       label="First action"
@@ -44,12 +45,13 @@ snapshots[
   kind="primary"
   label="Test"
   menu-alignment="bottom"
+  menu-background-token="layer"
   size="lg"
   tab-index="0"
 >
   <cds-menu
     size="lg"
-    style="inset-inline-start: 8px; inset-inline-end: initial; inset-block-start: 8px;"
+    style=""
   >
     <cds-menu-item
       label="First action"
@@ -78,12 +80,13 @@ snapshots[
   kind="primary"
   label="Nested"
   menu-alignment="bottom"
+  menu-background-token="layer"
   size="lg"
   tab-index="0"
 >
   <cds-menu
     size="lg"
-    style="inset-inline-start: 8px; inset-inline-end: initial; inset-block-start: 8px;"
+    style=""
   >
     <cds-menu-item
       aria-expanded="true"
@@ -121,110 +124,3 @@ snapshots['cds-menu-button should support xs size'] = `<cds-button
 </slot>
 `;
 /* end snapshot cds-menu-button should support xs size */
-snapshots['cds-menu-button should render and match snapshot'] =
-  `<cds-menu-button
-  kind="primary"
-  label="Actions"
-  menu-alignment="bottom"
-  menu-background-token="layer"
-  size="lg"
-  tab-index="0"
->
-  <cds-menu
-    size="lg"
-    style="inset-inline-start: 8px; inset-inline-end: initial; inset-block-start: 8px;"
-  >
-    <cds-menu-item
-      label="First action"
-      role="menuitem"
-      tabindex="0"
-    >
-    </cds-menu-item>
-    <cds-menu-item
-      label="Second action"
-      role="menuitem"
-      tabindex="0"
-    >
-    </cds-menu-item>
-    <cds-menu-item
-      aria-disabled="true"
-      disabled=""
-      label="Third action"
-      role="menuitem"
-      tabindex="-1"
-    >
-    </cds-menu-item>
-  </cds-menu>
-</cds-menu-button>
-`;
-/* end snapshot cds-menu-button should render and match snapshot */
-
-snapshots[
-  'cds-menu-button Children/slots and special menu content Snapshot variants should render with divider and danger and match snapshot'
-] = `<cds-menu-button
-  kind="primary"
-  label="Test"
-  menu-alignment="bottom"
-  menu-background-token="layer"
-  size="lg"
-  tab-index="0"
->
-  <cds-menu
-    size="lg"
-    style="inset-inline-start: 8px; inset-inline-end: initial; inset-block-start: 8px;"
-  >
-    <cds-menu-item
-      label="First action"
-      role="menuitem"
-      tabindex="0"
-    >
-    </cds-menu-item>
-    <cds-menu-item-divider role="separator">
-    </cds-menu-item-divider>
-    <cds-menu-item
-      class="cds--menu-item--danger"
-      kind="danger"
-      label="Danger"
-      role="menuitem"
-      tabindex="0"
-    >
-    </cds-menu-item>
-  </cds-menu>
-</cds-menu-button>
-`;
-/* end snapshot cds-menu-button Children/slots and special menu content Snapshot variants should render with divider and danger and match snapshot */
-
-snapshots[
-  'cds-menu-button Children/slots and special menu content Snapshot variants should render with nested menu and match snapshot'
-] = `<cds-menu-button
-  kind="primary"
-  label="Nested"
-  menu-alignment="bottom"
-  menu-background-token="layer"
-  size="lg"
-  tab-index="0"
->
-  <cds-menu
-    size="lg"
-    style="inset-inline-start: 8px; inset-inline-end: initial; inset-block-start: 8px;"
-  >
-    <cds-menu-item
-      aria-expanded="true"
-      aria-haspopup="true"
-      label="Export as"
-      role="menuitem"
-      tabindex="0"
-    >
-      <cds-menu-item-group slot="submenu">
-        <cds-menu-item
-          label="PDF"
-          role="menuitem"
-          tabindex="0"
-        >
-        </cds-menu-item>
-      </cds-menu-item-group>
-    </cds-menu-item>
-  </cds-menu>
-</cds-menu-button>
-`;
-/* end snapshot cds-menu-button Children/slots and special menu content Snapshot variants should render with nested menu and match snapshot */

packages/web-components/src/components/menu-button/__tests__/menu-button-test.js

@@ -280,6 +280,58 @@ describe('cds-menu-button', function () {
     });
   });
 
+  describe('internal close helpers', () => {
+    it('should close the menu and optionally restore focus', async () => {
+      const el = await fixture(menuButton);
+      const originalFocusTrigger = el._focusTrigger;
+      let focused = false;
+
+      el._focusTrigger = () => {
+        focused = true;
+      };
+
+      el._open = true;
+      el._closeMenu({ restoreFocus: true });
+      expect(el._open).to.be.false;
+      expect(focused).to.be.true;
+
+      focused = false;
+      el._open = true;
+      el._closeMenu({ restoreFocus: false });
+      expect(el._open).to.be.false;
+      expect(focused).to.be.false;
+
+      el._focusTrigger = originalFocusTrigger;
+    });
+
+    it('should respect menu closed events', async () => {
+      const el = await fixture(menuButton);
+      const menu = el.querySelector('cds-menu');
+
+      const closeCalls = [];
+      const originalCloseMenu = el._closeMenu;
+      el._closeMenu = (options) => {
+        closeCalls.push(options);
+      };
+
+      el._open = true;
+      el._handleMenuClosed({
+        target: menu,
+        detail: { triggerEventType: 'click' },
+      });
+      expect(closeCalls[0]).to.deep.equal({ restoreFocus: true });
+
+      el._open = true;
+      el._handleMenuClosed({
+        target: menu,
+        detail: { triggerEventType: 'focusout' },
+      });
+      expect(closeCalls[1]).to.deep.equal({ restoreFocus: false });
+
+      el._closeMenu = originalCloseMenu;
+    });
+  });
+
   // Note: Focus management in WC is limited by Shadow DOM, so this suite covers basic trigger behavior only
   describe('Keyboard and focus interaction', () => {
     it('should receive focus and open with Space/Enter', async () => {

packages/web-components/src/components/menu-button/menu-button.ts

@@ -36,9 +36,6 @@ class CDSMenuButton extends HostListenerMixin(LitElement) {
   @query(`${prefix}-button`)
   _triggerNode!: CDSButton;
 
-  @query(`${prefix}-menu`)
-  _menuNode!: CDSMenu;
-
   @property({ type: Boolean, reflect: true })
   private _open = false;
 
@@ -93,11 +90,35 @@ class CDSMenuButton extends HostListenerMixin(LitElement) {
   // eslint-disable-next-line @typescript-eslint/ban-ts-comment -- https://github.com/carbon-design-system/carbon/issues/20452
   // @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
   private _handleClick = (event: Event) => {
+    const { _triggerNode: trigger } = this;
+    if (!trigger) {
+      return;
+    }
+
     const path = event.composedPath();
-    if (path.includes(this._triggerNode)) {
-      this._open = !this._open;
+    if (path.includes(trigger)) {
+      if (this._open) {
+        this._closeMenu({ restoreFocus: true });
+      } else {
+        this._open = true;
+      }
     } else if (this._open) {
-      this._open = false;
+      this._closeMenu();
+    }
+  };
+
+  @HostListener('mousedown')
+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment -- https://github.com/carbon-design-system/carbon/issues/20452
+  // @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
+  private _handleMousedown = (event: MouseEvent) => {
+    const { _triggerNode: trigger } = this;
+    if (!trigger) {
+      return;
+    }
+
+    const path = event.composedPath();
+    if (path.includes(trigger)) {
+      event.preventDefault();
     }
   };
 
@@ -107,10 +128,46 @@ class CDSMenuButton extends HostListenerMixin(LitElement) {
   private _handleBlur = ({ relatedTarget }: FocusEvent) => {
     // Close the menu if the focus moves outside the menu button or menu
     if (!this.contains(relatedTarget as Node)) {
-      this._open = false;
+      this._closeMenu();
     }
   };
 
+  @HostListener('keydown')
+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment -- https://github.com/carbon-design-system/carbon/issues/20452
+  // @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
+  private _handleKeydown = (event: KeyboardEvent) => {
+    if (!this._open || event.key !== 'Escape') {
+      return;
+    }
+
+    const { _triggerNode: trigger } = this;
+    if (!trigger) {
+      return;
+    }
+
+    const path = event.composedPath();
+    if (path.includes(trigger)) {
+      event.stopPropagation();
+      event.preventDefault();
+      this._closeMenu({ restoreFocus: true });
+    }
+  };
+
+  @HostListener(`${prefix}-menu-closed`)
+  // eslint-disable-next-line @typescript-eslint/ban-ts-comment -- https://github.com/carbon-design-system/carbon/issues/20452
+  // @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
+  private _handleMenuClosed = (
+    event: CustomEvent<{ triggerEventType?: string }>
+  ) => {
+    const menu = this.querySelector(`${prefix}-menu`);
+    if (!menu || event.target !== menu || !this._open) {
+      return;
+    }
+
+    const shouldRestoreFocus = event.detail?.triggerEventType !== 'focusout';
+    this._closeMenu({ restoreFocus: shouldRestoreFocus });
+  };
+
   updated(changedProperties) {
     const menu = this.querySelector(`${prefix}-menu`) as CDSMenu;
 
@@ -164,6 +221,28 @@ class CDSMenuButton extends HostListenerMixin(LitElement) {
   }
 
   static styles = styles;
+
+  private _closeMenu({
+    restoreFocus = false,
+  }: { restoreFocus?: boolean } = {}) {
+    if (!this._open) {
+      return;
+    }
+
+    this._open = false;
+
+    if (restoreFocus) {
+      this._focusTrigger();
+    }
+  }
+
+  private _focusTrigger() {
+    if (!this._triggerNode || typeof this._triggerNode.focus !== 'function') {
+      return;
+    }
+
+    this._triggerNode.focus();
+  }
 }
 
 export default CDSMenuButton;

packages/web-components/src/components/menu/__tests__/menu-item-test.js

@@ -191,6 +191,22 @@ describe('cds-menu-item', () => {
       // Restore original method
       el._handleClick = originalHandleClick;
     });
+
+    it('should dispatch a close request when activated without a submenu', async () => {
+      const el = await fixture(
+        html`<cds-menu-item label="Test Item"></cds-menu-item>`
+      );
+
+      const events = [];
+      el.addEventListener('cds-menu-close-root-request', (event) => {
+        events.push(event);
+      });
+
+      el.dispatchEvent(new MouseEvent('click', { bubbles: true }));
+
+      expect(events.length).to.equal(1);
+      expect(events[0].detail.triggerEvent).to.be.instanceOf(MouseEvent);
+    });
   });
 
   describe('submenu positioning', () => {