Merge remote-tracking branch 'origin/main' into r/submenu-items-use-d…

…ropdownmenu-portal
iTwin · Mar 6, 2025 · c8f631d · c8f631d
2 parents f420493 + 4c78daf
commit c8f631d
Show file tree

Hide file tree

Showing 7 changed files with 265 additions and 108 deletions.
diff --git a/.changeset/icy-plants-play.md b/.changeset/icy-plants-play.md
@@ -0,0 +1,5 @@
+---
+'@itwin/itwinui-react': patch
+---
+
+Fixed bug where nested `MenuItem`s in `Tile.MoreOptions` were not closing when clicked.
diff --git a/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx b/packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx
@@ -99,40 +99,67 @@ const DropdownMenuContent = React.forwardRef((props, forwardedRef) => {
     onVisibleChange,
   );
 
+  const close = React.useCallback(() => {
+    setVisible(false);
+  }, [setVisible]);
+
   const menuContent = React.useMemo(() => {
     if (typeof menuItems === 'function') {
-      return menuItems(() => setVisible(false));
+      return menuItems(close);
     }
     return menuItems;
-  }, [menuItems, setVisible]);
+  }, [close, menuItems]);
+
+  const dropdownMenuContextValue = React.useMemo(() => ({ close }), [close]);
 
   return (
-    <Menu
-      trigger={children}
-      onKeyDown={mergeEventHandlers(props.onKeyDown, (e) => {
-        if (e.defaultPrevented) {
-          return;
-        }
-        if (e.key === 'Tab') {
-          setVisible(false);
-        }
-      })}
-      role={role}
-      ref={forwardedRef}
-      portal={portal}
-      popoverProps={React.useMemo(
-        () => ({
-          placement,
-          matchWidth,
-          visible,
-          onVisibleChange: setVisible,
-          middleware,
-        }),
-        [matchWidth, middleware, placement, setVisible, visible],
-      )}
-      {...rest}
-    >
-      {menuContent}
-    </Menu>
+    <DropdownMenuContext.Provider value={dropdownMenuContextValue}>
+      <Menu
+        trigger={children}
+        onKeyDown={mergeEventHandlers(props.onKeyDown, (e) => {
+          if (e.defaultPrevented) {
+            return;
+          }
+          if (e.key === 'Tab') {
+            setVisible(false);
+          }
+        })}
+        role={role}
+        ref={forwardedRef}
+        portal={portal}
+        popoverProps={React.useMemo(
+          () => ({
+            placement,
+            matchWidth,
+            visible,
+            onVisibleChange: setVisible,
+            middleware,
+          }),
+          [matchWidth, middleware, placement, setVisible, visible],
+        )}
+        {...rest}
+      >
+        {menuContent}
+      </Menu>
+    </DropdownMenuContext.Provider>
   );
 }) as PolymorphicForwardRefComponent<'div', DropdownMenuProps>;
+
+// ----------------------------------------------------------------------------
+
+export const DropdownMenuContext = React.createContext<
+  | {
+      close: () => void;
+    }
+  | undefined
+>(undefined);
+
+/**
+ * @private
+ * Wraps around a `DropdownMenu`.
+ *
+ * If `true`, closes the `DropdownMenu` when any descendant `MenuItem` is clicked.
+ */
+export const DropdownMenuCloseOnClickContext = React.createContext<
+  boolean | undefined
+>(undefined);
diff --git a/packages/itwinui-react/src/core/Menu/Menu.tsx b/packages/itwinui-react/src/core/Menu/Menu.tsx
@@ -251,51 +251,51 @@ export const Menu = React.forwardRef((props, ref) => {
     () => undefined,
   );
 
-  const popoverGetItemProps: PopoverGetItemProps = ({
-    focusableItemIndex,
-    userProps,
-  }) => {
-    return getItemProps({
-      ...userProps,
-      // Roving tabIndex
-      tabIndex:
-        activeIndex != null &&
-        activeIndex >= 0 &&
-        focusableItemIndex != null &&
-        focusableItemIndex >= 0 &&
-        activeIndex === focusableItemIndex
-          ? 0
-          : -1,
-      onFocus: mergeEventHandlers(userProps?.onFocus, () => {
-        // Set hasFocusedNodeInSubmenu in a microtask to ensure the submenu stays open reliably.
-        // E.g. Even when hovering into it rapidly.
-        queueMicrotask(() => {
-          setHasFocusedNodeInSubmenu(true);
-        });
-        tree?.events.emit('onNodeFocused', {
-          nodeId: nodeId,
-          parentId: parentId,
-        });
-      }),
-
-      // useListNavigation sets focusItemOnHover to false, since it doesn't work for us.
-      // Thus, we need to manually emulate the "focus on hover" behavior.
-      onMouseEnter: mergeEventHandlers(userProps?.onMouseEnter, (event) => {
-        // Updating the activeIndex will result in useListNavigation focusing the item.
-        if (focusableItemIndex != null && focusableItemIndex >= 0) {
-          setActiveIndex(focusableItemIndex);
-        }
-
-        // However, useListNavigation only focuses the item when the activeIndex changes.
-        // So, if we re-hover the parent MenuItem of an open submenu, the activeIndex won't change,
-        // and thus the hovered MenuItem won't be focused.
-        // As a result, we need to explicitly focus the item manually.
-        if (event.target === event.currentTarget) {
-          event.currentTarget.focus();
-        }
-      }),
-    });
-  };
+  const popoverGetItemProps: PopoverGetItemProps = React.useCallback(
+    ({ focusableItemIndex, userProps }) => {
+      return getItemProps({
+        ...userProps,
+        // Roving tabIndex
+        tabIndex:
+          activeIndex != null &&
+          activeIndex >= 0 &&
+          focusableItemIndex != null &&
+          focusableItemIndex >= 0 &&
+          activeIndex === focusableItemIndex
+            ? 0
+            : -1,
+        onFocus: mergeEventHandlers(userProps?.onFocus, () => {
+          // Set hasFocusedNodeInSubmenu in a microtask to ensure the submenu stays open reliably.
+          // E.g. Even when hovering into it rapidly.
+          queueMicrotask(() => {
+            setHasFocusedNodeInSubmenu(true);
+          });
+          tree?.events.emit('onNodeFocused', {
+            nodeId: nodeId,
+            parentId: parentId,
+          });
+        }),
+
+        // useListNavigation sets focusItemOnHover to false, since it doesn't work for us.
+        // Thus, we need to manually emulate the "focus on hover" behavior.
+        onMouseEnter: mergeEventHandlers(userProps?.onMouseEnter, (event) => {
+          // Updating the activeIndex will result in useListNavigation focusing the item.
+          if (focusableItemIndex != null && focusableItemIndex >= 0) {
+            setActiveIndex(focusableItemIndex);
+          }
+
+          // However, useListNavigation only focuses the item when the activeIndex changes.
+          // So, if we re-hover the parent MenuItem of an open submenu, the activeIndex won't change,
+          // and thus the hovered MenuItem won't be focused.
+          // As a result, we need to explicitly focus the item manually.
+          if (event.target === event.currentTarget) {
+            event.currentTarget.focus();
+          }
+        }),
+      });
+    },
+    [activeIndex, getItemProps, nodeId, parentId, tree?.events],
+  );
 
   const reference = cloneElementWithRef(trigger, (triggerChild) =>
     getReferenceProps(
@@ -327,7 +327,15 @@ export const Menu = React.forwardRef((props, ref) => {
 
   return (
     <>
-      <MenuContext.Provider value={{ popoverGetItemProps, focusableElements }}>
+      <MenuContext.Provider
+        value={React.useMemo(
+          () => ({
+            popoverGetItemProps,
+            focusableElements,
+          }),
+          [focusableElements, popoverGetItemProps],
+        )}
+      >
         <MenuPortalContext.Provider value={portal}>
           <PopoverOpenContext.Provider value={popover.open}>
             {reference}

diff --git a/packages/itwinui-react/src/core/Menu/MenuItem.tsx b/packages/itwinui-react/src/core/Menu/MenuItem.tsx
@@ -14,6 +14,10 @@ import { Menu, MenuContext } from './Menu.js';
 import { ListItem } from '../List/ListItem.js';
 import type { ListItemOwnProps } from '../List/ListItem.js';
 import cx from 'classnames';
+import {
+  DropdownMenuCloseOnClickContext,
+  DropdownMenuContext,
+} from '../DropdownMenu/DropdownMenu.js';
 
 export type MenuItemProps = {
   /**
@@ -111,6 +115,10 @@ export const MenuItem = React.forwardRef((props, forwardedRef) => {
   }
 
   const parentMenu = React.useContext(MenuContext);
+  const dropdownMenu = React.useContext(DropdownMenuContext);
+  const shouldCloseMenuOnClick = React.useContext(
+    DropdownMenuCloseOnClickContext,
+  );
 
   const menuItemRef = React.useRef<HTMLElement>(null);
   const submenuId = useId();
@@ -134,6 +142,10 @@ export const MenuItem = React.forwardRef((props, forwardedRef) => {
       return;
     }
 
+    if (shouldCloseMenuOnClick) {
+      dropdownMenu?.close();
+    }
+
     onClickProp?.(value);
   };
 

diff --git a/packages/itwinui-react/src/core/Tile/Tile.tsx b/packages/itwinui-react/src/core/Tile/Tile.tsx
@@ -15,7 +15,10 @@ import {
   Box,
 } from '../../utils/index.js';
 import type { PolymorphicForwardRefComponent } from '../../utils/index.js';
-import { DropdownMenu } from '../DropdownMenu/DropdownMenu.js';
+import {
+  DropdownMenu,
+  DropdownMenuCloseOnClickContext,
+} from '../DropdownMenu/DropdownMenu.js';
 import { IconButton } from '../Buttons/IconButton.js';
 import { ProgressRadial } from '../ProgressIndicators/ProgressRadial.js';
 import { LinkAction } from '../LinkAction/LinkAction.js';
@@ -384,40 +387,33 @@ const TileMoreOptions = React.forwardRef((props, forwardedRef) => {
   const [isMenuVisible, setIsMenuVisible] = React.useState(false);
 
   return (
-    <Box
-      className={cx(
-        'iui-tile-more-options',
-        {
-          'iui-visible': isMenuVisible,
-        },
-        className,
-      )}
-      ref={forwardedRef}
-      {...rest}
-    >
-      <DropdownMenu
-        onVisibleChange={setIsMenuVisible}
-        menuItems={(close) =>
-          children?.map((option: React.ReactElement<any>) =>
-            React.cloneElement(option, {
-              onClick: (value: unknown) => {
-                close();
-                option.props.onClick?.(value);
-              },
-            }),
-          )
-        }
+    <DropdownMenuCloseOnClickContext.Provider value={true}>
+      <Box
+        className={cx(
+          'iui-tile-more-options',
+          {
+            'iui-visible': isMenuVisible,
+          },
+          className,
+        )}
+        ref={forwardedRef}
+        {...rest}
       >
-        <IconButton
-          styleType='borderless'
-          size='small'
-          aria-label='More options'
-          {...buttonProps}
+        <DropdownMenu
+          onVisibleChange={setIsMenuVisible}
+          menuItems={children as React.ReactElement<any>[]}
         >
-          <SvgMore />
-        </IconButton>
-      </DropdownMenu>
-    </Box>
+          <IconButton
+            styleType='borderless'
+            size='small'
+            aria-label='More options'
+            {...buttonProps}
+          >
+            <SvgMore />
+          </IconButton>
+        </DropdownMenu>
+      </Box>
+    </DropdownMenuCloseOnClickContext.Provider>
   );
 }) as PolymorphicForwardRefComponent<'div', TileMoreOptionsOwnProps>;
 if (process.env.NODE_ENV === 'development') {