fix: Close nested Menus when hovering over other items

cypress/component/Menu.spec.tsx

@@ -1,5 +1,7 @@
 import {Basic} from '../../modules/react/menu/stories/examples/Basic';
 import {MenuWithFallbackPlacements} from '../../modules/react/menu/stories/examples/MenuWithFallbackPlacements';
+import {NestedDynamic} from '../../modules/react/menu/stories/examples/NestedDynamic';
+import {NestedSiblings} from '../../modules/react/menu/stories/examples/NestedSiblings';
 
 describe('Menu', () => {
   context(`given the [Components/Popups/Menu, Basic] story is rendered`, () => {
@@ -183,6 +185,27 @@ describe('Menu', () => {
         });
       });
 
+      context('when a disabled item has focus and the menu is closed and reopened', () => {
+        beforeEach(() => {
+          cy.findByRole('menuitem', {name: 'First Item'})
+            .should('be.focused')
+            .realType('{uparrow}');
+          cy.findByRole('menuitem', {name: 'Fourth Item'}).should('be.focused');
+          cy.focused().realType('{esc}');
+          cy.findByRole('menu').should('not.exist');
+          cy.findByRole('button', {name: 'Open Menu'}).click();
+          cy.findByRole('menu').should('be.visible');
+        });
+
+        it('should move focus to the first item instead of the disabled item', () => {
+          cy.findByRole('menuitem', {name: 'First Item'}).should('be.focused');
+        });
+
+        it('should not leave focus on the disabled item', () => {
+          cy.findByRole('menuitem', {name: 'Fourth Item'}).should('not.be.focused');
+        });
+      });
+
       context('when the enter key is pressed', () => {
         beforeEach(() => {
           cy.findByRole('menu').should('exist');
@@ -209,6 +232,141 @@ describe('Menu', () => {
     });
   });
 
+  context(`given the [Components/Popups/Menu, NestedSiblings] story is rendered`, () => {
+    beforeEach(() => {
+      cy.mount(<NestedSiblings />);
+      cy.findByRole('button', {name: 'Open Menu'}).click();
+      cy.findByRole('menu').should('be.visible');
+    });
+
+    context('when the "Second Item" submenu is opened by hovering', () => {
+      beforeEach(() => {
+        cy.findByRole('menuitem', {name: 'Second Item'}).realHover();
+        cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.visible');
+      });
+
+      it('should set aria-expanded to true on "Second Item"', () => {
+        cy.findByRole('menuitem', {name: 'Second Item'}).should(
+          'have.attr',
+          'aria-expanded',
+          'true'
+        );
+      });
+
+      context('when the sibling "Third Item" is then hovered', () => {
+        beforeEach(() => {
+          cy.findByRole('menuitem', {name: 'Third Item'}).realHover();
+          cy.findByRole('menuitem', {name: 'Third: First Sub Item'}).should('be.visible');
+        });
+
+        it('should close the "Second Item" submenu', () => {
+          cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('not.exist');
+        });
+
+        it('should set aria-expanded to false on "Second Item"', () => {
+          cy.findByRole('menuitem', {name: 'Second Item'}).should(
+            'have.attr',
+            'aria-expanded',
+            'false'
+          );
+        });
+
+        it('should leave only the root menu and one submenu open', () => {
+          cy.findAllByRole('menu').should('have.length', 2);
+        });
+
+        context('when the original "Second Item" is hovered again', () => {
+          beforeEach(() => {
+            cy.findByRole('menuitem', {name: 'Second Item'}).realHover();
+            cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.visible');
+          });
+
+          it('should close the "Third Item" submenu', () => {
+            cy.findByRole('menuitem', {name: 'Third: First Sub Item'}).should('not.exist');
+          });
+
+          it('should leave only the root menu and one submenu open', () => {
+            cy.findAllByRole('menu').should('have.length', 2);
+          });
+        });
+      });
+    });
+
+    context('when a submenu is opened with the right arrow key', () => {
+      beforeEach(() => {
+        cy.findByRole('menuitem', {name: 'First Item'}).should('be.focused');
+        cy.focused().realType('{downarrow}');
+        cy.findByRole('menuitem', {name: 'Second Item'}).should('be.focused');
+        cy.focused().realType('{rightarrow}');
+      });
+
+      it('should open the submenu', () => {
+        cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.visible');
+      });
+
+      it('should keep the submenu open and move focus into it', () => {
+        cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.focused');
+      });
+
+      context('when the left arrow key is pressed', () => {
+        beforeEach(() => {
+          cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('be.focused');
+          cy.focused().realType('{leftarrow}');
+        });
+
+        it('should close the submenu', () => {
+          cy.findByRole('menuitem', {name: 'Second: First Sub Item'}).should('not.exist');
+        });
+
+        it('should return focus to the "Second Item" target', () => {
+          cy.findByRole('menuitem', {name: 'Second Item'}).should('be.focused');
+        });
+      });
+    });
+  });
+
+  context(`given the [Components/Popups/Menu, NestedDynamic] story is rendered`, () => {
+    beforeEach(() => {
+      cy.mount(<NestedDynamic />);
+      cy.findByRole('button', {name: 'Open Menu'}).click();
+      cy.findByRole('menu').should('be.visible');
+    });
+
+    context('when hovering deeper into a chain of nested submenus', () => {
+      beforeEach(() => {
+        cy.findByRole('menuitem', {name: 'Second Item'}).realHover();
+        cy.findByRole('menuitem', {name: 'Second Sub Item'}).should('be.visible');
+        cy.findByRole('menuitem', {name: 'Second Sub Item'}).realHover();
+        cy.findByRole('menuitem', {name: 'Second Sub Sub Item'}).should('be.visible');
+      });
+
+      it('should keep the ancestor submenu open', () => {
+        cy.findByRole('menuitem', {name: 'Second Item'}).should(
+          'have.attr',
+          'aria-expanded',
+          'true'
+        );
+      });
+
+      it('should keep every menu in the open chain visible', () => {
+        cy.findAllByRole('menu').should('have.length', 3);
+      });
+
+      // Re-entering an item that is already part of the open trail should not collapse the trail,
+      // matching the platform menu convention of only closing when a sibling is hovered.
+      context('when hovering back onto the ancestor item in the open trail', () => {
+        beforeEach(() => {
+          cy.findByRole('menuitem', {name: 'Second Item'}).realHover();
+        });
+
+        it('should keep the open trail intact', () => {
+          cy.findByRole('menuitem', {name: 'Second Sub Sub Item'}).should('be.visible');
+          cy.findAllByRole('menu').should('have.length', 3);
+        });
+      });
+    });
+  });
+
   context(`given the [Testing/Popups/Menu, MenuWithFallbackPlacements] example is rendered`, () => {
     beforeEach(() => {
       cy.mount(<MenuWithFallbackPlacements />);

modules/react/collection/index.ts

@@ -18,7 +18,8 @@ export * from './lib/useListItemAllowChildStrings';
 export * from './lib/useListItemRemoveOnDeleteKey';
 export * from './lib/focusOnCurrentCursor';
 export * from './lib/listItemRemove';
-export {ListBox, type ListBoxProps} from './lib/ListBox';
+export * from './lib/isElementDisabled';
+export {ListBox, type ListBoxProps, listBoxContainerStencil} from './lib/ListBox';
 export {keyboardEventToCursorEvents} from './lib/keyUtils';
 export {
   singleSelectionManager,

modules/react/collection/lib/ListBox.tsx

@@ -51,7 +51,7 @@ export const useListBox = createElemPropsHook(useListModel)(model => {
   };
 });
 
-const listBoxContainerStencil = createStencil({
+export const listBoxContainerStencil = createStencil({
   parts: {
     listBoxContainer: 'list-box-container',
   },

modules/react/collection/lib/isElementDisabled.ts

@@ -0,0 +1,16 @@
+/**
+ * Checks whether an element is disabled, either via the native `disabled` DOM property (set, for
+ * example, by `useListItemRegister` when an item's id is included in `state.nonInteractiveIds`) or
+ * via the `aria-disabled="true"` attribute (set by consumers directly on an item, e.g.
+ * `<Menu.Item aria-disabled>`).
+ *
+ * Collection items should use this check instead of testing `aria-disabled` alone so that items
+ * disabled through either mechanism are consistently blocked from activating (click, Enter/Space,
+ * hover-intent, etc.).
+ */
+export const isElementDisabled = (element: Element | null | undefined): boolean => {
+  if (!element) {
+    return false;
+  }
+  return element.getAttribute('aria-disabled') === 'true' || element.hasAttribute('disabled');
+};

modules/react/collection/lib/useListItemSelect.tsx

@@ -2,6 +2,7 @@ import React from 'react';
 
 import {createElemPropsHook} from '@workday/canvas-kit-react/common';
 
+import {isElementDisabled} from './isElementDisabled';
 import {useListModel} from './useListModel';
 
 /**
@@ -23,12 +24,11 @@ export const useListItemSelect = createElemPropsHook(useListModel)((
 ) => {
   const name = elemProps['data-id'] || '';
   const onClick = (event: React.MouseEvent<HTMLElement>) => {
-    if (
-      !state.nonInteractiveIds.includes(name) &&
-      event.currentTarget.getAttribute('aria-disabled') !== 'true'
-    ) {
-      events.select({id: name});
+    if (isElementDisabled(event.currentTarget) || state.nonInteractiveIds.includes(name)) {
+      return null;
     }
+    events.select({id: name});
+    return undefined;
   };
 
   return {onClick};

modules/react/menu/lib/MenuCard.tsx

@@ -1,14 +1,15 @@
 import * as React from 'react';
 
 import {Card} from '@workday/canvas-kit-react/card';
+import {listBoxContainerStencil} from '@workday/canvas-kit-react/collection';
 import {
   ExtractProps,
   createElemPropsHook,
   createSubcomponent,
 } from '@workday/canvas-kit-react/common';
 import {mergeStyles} from '@workday/canvas-kit-react/layout';
 import {getTransformFromPlacement} from '@workday/canvas-kit-react/popup';
-import {calc, createStencil, px2rem} from '@workday/canvas-kit-styling';
+import {calc, createStencil, cssVar, px2rem} from '@workday/canvas-kit-styling';
 import {system} from '@workday/canvas-tokens-web';
 
 import {useMenuModel} from './useMenuModel';
@@ -42,12 +43,22 @@ export const menuCardStencil = createStencil({
     maxWidth: calc.subtract('100vw', system.legacy.size.sm),
     boxShadow: system.depth[3],
     minWidth,
-    maxHeight,
+    maxHeight: cssVar(maxHeight, '60vh'),
     transformOrigin: `${transformOriginVertical} ${transformOriginHorizontal}`,
     // Allow overriding of animation in special cases
     '.wd-no-animation &': {
       animation: 'none',
     },
+    [`&:where(:has(${listBoxContainerStencil.parts.listBoxContainer.selector}))`]: {
+      overflow: 'hidden',
+    },
+    [`& :where(${listBoxContainerStencil.parts.listBoxContainer.selector})`]: {
+      borderRadius: system.legacy.shape.xxl,
+      // Card is a flex column container. Without this, a flex child won't shrink below its
+      // content size, so `maxHeight` on the Card would be ignored and content would overflow
+      // instead of scrolling inside the list-box-container.
+      minHeight: 0,
+    },
   }),
 });
 

modules/react/menu/lib/MenuItem.tsx

@@ -2,11 +2,13 @@ import * as React from 'react';
 
 import {
   isCursor,
+  isElementDisabled,
   useListItemRegister,
   useListItemRovingFocus,
   useListItemSelect,
 } from '@workday/canvas-kit-react/collection';
 import {
+  changeFocus,
   composeHooks,
   createComponent,
   createElemPropsHook,
@@ -179,7 +181,12 @@ export const useMenuItemArrowReturn = createElemPropsHook(useMenuModel)(model =>
     onKeyDown(event: React.KeyboardEvent) {
       const styles = getComputedStyle(event.currentTarget);
       if (event.key === 'ArrowLeft' && styles.direction === 'ltr' && model.UNSTABLE_parentModel) {
+        event.preventDefault();
+        const target = model.state.targetRef.current as HTMLElement | null;
         model.events.hide(event);
+        requestAnimationFrame(() => {
+          changeFocus(target);
+        });
       }
     },
   };
@@ -192,6 +199,27 @@ export const useMenuItemFocus = createElemPropsHook(useMenuModel)((
 ) => {
   const {localRef, elementRef} = useLocalRef(ref as React.Ref<HTMLElement>);
   const id = elemProps['data-id'];
+
+  // A menu keeps its cursor between openings, so reopening would restore focus to a disabled item.
+  // Items remount whenever the menu opens, which distinguishes reopening from navigating onto a
+  // disabled item while the menu is already open. Clearing the cursor lets the roving focus
+  // fallback in `useListItemRovingFocus` move focus to the first item. The check waits for the
+  // first render where `id` is known, since items without an explicit `data-id` register it later.
+  const hasCheckedDisabledCursor = React.useRef(false);
+  React.useLayoutEffect(() => {
+    if (hasCheckedDisabledCursor.current || !id) {
+      return;
+    }
+    hasCheckedDisabledCursor.current = true;
+
+    const isItemDisabled = isElementDisabled(localRef.current);
+
+    if (model.state.mode === 'single' && isCursor(model.state, id) && isItemDisabled) {
+      model.events.goToFirst();
+    }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [id]);
+
   // focus on the item with the cursor
   React.useLayoutEffect(() => {
     if (model.state.mode === 'single') {
@@ -227,11 +255,12 @@ export const useMenuItem = composeHooks(
       onClick:
         model.state.mode === 'single'
           ? (event: React.SyntheticEvent) => {
-              // only hide if the item isn't disabled
-              if (event.currentTarget.getAttribute('aria-disabled') !== 'true') {
-                model.events.hide(event);
-                hideParent(model);
+              if (isElementDisabled(event.currentTarget as Element)) {
+                return null;
               }
+              model.events.hide(event);
+              hideParent(model);
+              return undefined;
             }
           : undefined,
     };

modules/react/menu/lib/MenuList.tsx

@@ -54,12 +54,13 @@ export const MenuList = createSubcomponent('div')({
   displayName: 'Menu.List',
   modelHook: useMenuModel,
   elemPropsHook: useMenuList,
-})<MenuListProps>(({children, ...elemProps}, Element, model) => {
+})<MenuListProps>(({children, maxHeight, ...elemProps}, Element, model) => {
   return (
     <ListBox
       as={Element}
       model={model}
       marginY={system.legacy.gap.none}
+      maxHeight={maxHeight ?? '100%'}
       {...handleCsProp(elemProps, menuListStencil({orientation: model.state.orientation}))}
     >
       {children}