Menu Props [LG-3259]

.changeset/two-deers-matter.md

@@ -0,0 +1,6 @@
+---
+'@leafygreen-ui/menu': major
+---
+
+- Updates Menu props to extend `div` props
+- Moves `...rest` prop spread onto the `Popover` div (previously props were spread on the inner `ul` element)

packages/menu/package.json

@@ -22,7 +22,7 @@
     "access": "public"
   },
   "dependencies": {
-    "@leafygreen-ui/descendants": "^0.1.1",
+    "@leafygreen-ui/descendants": "^0.2.0",
     "@leafygreen-ui/emotion": "^4.0.8",
     "@leafygreen-ui/hooks": "^8.1.3",
     "@leafygreen-ui/icon": "^12.5.4",
@@ -50,7 +50,7 @@
   },
   "devDependencies": {
     "@leafygreen-ui/button": "^21.2.0",
-    "@leafygreen-ui/testing-lib": "^0.5.0",
+    "@leafygreen-ui/testing-lib": "^0.6.0",
     "@lg-tools/storybook-utils": "^0.1.1",
     "@storybook/react": "^7.6.17"
   }

packages/menu/src/Menu.spec.tsx

@@ -1,7 +1,6 @@
 import React from 'react';
 import {
   act,
-  fireEvent,
   render,
   waitFor,
   waitForElementToBeRemoved,
@@ -74,7 +73,10 @@ function renderMenu(
   async function openMenu() {
     userEvent.click(triggerEl);
     const menuElements = await findMenuElements();
-    fireEvent.transitionEnd(menuElements.menuEl as Element); // JSDOM does not automatically fire these events
+    await waitForTransition(menuElements.menuEl);
+    await waitFor(() => {
+      expect(menuElements.menuItemElements[0]).toHaveFocus();
+    });
     return menuElements;
   }
 
@@ -159,9 +161,18 @@ describe('packages/menu', () => {
       const { triggerEl, findMenuElements } = renderMenu({});
       userEvent.click(triggerEl);
       const { menuEl, menuItemElements } = await findMenuElements();
+      await waitForTransition(menuEl);
+      await waitFor(() => {
+        expect(menuItemElements[0]).toHaveFocus();
+      });
+    });
+
+    test('First item is focused when { usePortal: false }', async () => {
+      const { triggerEl, findMenuElements } = renderMenu({ usePortal: false });
+      userEvent.click(triggerEl);
+      const { menuEl, menuItemElements } = await findMenuElements();
+      await waitForTransition(menuEl);
       await waitFor(() => {
-        // JSDOM does not automatically fire these events
-        fireEvent.transitionEnd(menuEl as Element);
         expect(menuItemElements[0]).toHaveFocus();
       });
     });
@@ -220,7 +231,7 @@ describe('packages/menu', () => {
       if (key === 'tab') {
         userEvent.tab();
       } else {
-        userEvent.type(menu, `{${key}}`);
+        userEvent.keyboard(`{${key}}`);
       }
     };
 
@@ -249,11 +260,16 @@ describe('packages/menu', () => {
         const { openMenu, triggerEl } = renderMenu({
           usePortal: false,
         });
-        const { menuEl } = await openMenu();
-
+        const { menuEl, menuItemElements } = await openMenu();
+        await waitFor(() => {
+          expect(menuEl).toBeInTheDocument();
+          expect(menuItemElements[0]).toHaveFocus();
+        });
         userEventInteraction(menuEl!, key);
         await waitForElementToBeRemoved(menuEl);
-        expect(triggerEl).toHaveFocus();
+        await waitFor(() => {
+          expect(triggerEl).toHaveFocus();
+        });
       });
     });
 
@@ -348,7 +364,7 @@ describe('packages/menu', () => {
 
       expect(firstItem).toHaveFocus();
 
-      userEvent.keyboard('[Enter]');
+      userEvent.type(menuEl!, '[Enter]');
 
       await act(async () => await waitForTimeout());
       expect(menuEl).toBeInTheDocument();
@@ -364,7 +380,7 @@ describe('packages/menu', () => {
 
       expect(firstItem).toHaveFocus();
 
-      userEvent.keyboard('[Space]');
+      userEvent.type(menuEl!, '[Space]');
 
       await act(async () => await waitForTimeout());
       expect(menuEl).toBeInTheDocument();

packages/menu/src/Menu.stories.tsx

@@ -55,7 +55,6 @@ export default {
         'setOpen',
         'as',
         'portalRef',
-        'usePortal',
       ],
     },
     chromatic: {
@@ -82,6 +81,9 @@ export default {
     renderDarkMenu: {
       control: 'boolean',
     },
+    usePortal: {
+      control: 'boolean',
+    },
   },
 } satisfies StoryMetaType<typeof Menu>;
 

packages/menu/src/Menu/Menu.tsx

@@ -1,4 +1,10 @@
-import React, { useCallback, useRef, useState } from 'react';
+import React, {
+  KeyboardEventHandler,
+  MouseEventHandler,
+  useCallback,
+  useRef,
+  useState,
+} from 'react';
 import PropTypes from 'prop-types';
 
 import {
@@ -7,7 +13,7 @@ import {
   useInitDescendants,
 } from '@leafygreen-ui/descendants';
 import { css, cx } from '@leafygreen-ui/emotion';
-import { useBackdropClick, useEventListener } from '@leafygreen-ui/hooks';
+import { useBackdropClick, useForwardedRef } from '@leafygreen-ui/hooks';
 import { useDarkMode } from '@leafygreen-ui/leafygreen-provider';
 import { isDefined, keyMap, Theme } from '@leafygreen-ui/lib';
 import Popover, { Align, Justify } from '@leafygreen-ui/popover';
@@ -59,6 +65,8 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu(
     setOpen: controlledSetOpen,
     darkMode: darkModeProp,
     renderDarkMenu = true,
+    onClick,
+    onKeyDown,
     children,
     className,
     refEl,
@@ -70,11 +78,11 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu(
     popoverZIndex,
     ...rest
   }: MenuProps,
-  forwardRef,
+  fwdRef,
 ) {
   const { theme, darkMode } = useDarkMode(darkModeProp);
 
-  const popoverRef = useRef<HTMLUListElement | null>(null);
+  const popoverRef = useForwardedRef(fwdRef, null);
   const triggerRef = useRef<HTMLElement>(null);
 
   const [uncontrolledOpen, uncontrolledSetOpen] = useState(initialOpen);
@@ -112,6 +120,12 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu(
     },
   );
 
+  const handleMenuClick: MouseEventHandler<HTMLDivElement> = e => {
+    // Need to stop propagation, otherwise Menu will closed automatically when clicked
+    e.stopPropagation();
+    onClick?.(e);
+  };
+
   // Callback fired when the popover transition finishes.
   // Handling on this event ensures that the `descendants` elements
   // exist in the DOM before attempting to set `focus`
@@ -120,7 +134,7 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu(
   };
 
   // Fired on global keyDown event
-  function handleKeyDown(e: KeyboardEvent) {
+  const handleKeyDown: KeyboardEventHandler<HTMLDivElement> = e => {
     switch (e.key) {
       case keyMap.ArrowDown:
         e.preventDefault(); // Prevents page scrolling
@@ -150,9 +164,8 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu(
         }
         break;
     }
-  }
-
-  useEventListener('keydown', handleKeyDown, { enabled: open });
+    onKeyDown?.(e);
+  };
 
   const popoverProps = {
     popoverZIndex,
@@ -185,14 +198,18 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu(
       >
         <Popover
           key="popover"
+          ref={popoverRef}
           active={open}
           align={align}
           justify={justify}
           refEl={refEl}
           adjustOnMutation={adjustOnMutation}
+          onClick={handleMenuClick}
+          onKeyDown={handleKeyDown}
           onEntered={handlePopoverOpen}
           data-testid={LGIDs.root}
           data-lgid={LGIDs.root}
+          {...rest}
           {...popoverProps}
         >
           <div
@@ -210,17 +227,8 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu(
               },
               className,
             )}
-            ref={forwardRef}
           >
-            {/* Need to stop propagation, otherwise Menu will closed automatically when clicked */}
-            {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events*/}
-            <ul
-              {...rest}
-              className={scrollContainerStyle}
-              role="menu"
-              onClick={e => e.stopPropagation()}
-              ref={popoverRef}
-            >
+            <ul className={scrollContainerStyle} role="menu">
               {children}
             </ul>
           </div>

packages/menu/src/Menu/Menu.types.ts

@@ -1,4 +1,4 @@
-import { ReactElement, ReactNode } from 'react';
+import { ComponentPropsWithoutRef, ReactElement, ReactNode } from 'react';
 
 import {
   InferredPolymorphicPropsWithRef,
@@ -12,7 +12,9 @@ export type SubMenuType = ReactElement<
   InferredPolymorphicPropsWithRef<PolymorphicAs, SubMenuProps>
 >;
 
-export interface MenuProps extends Omit<PopoverProps, 'active'> {
+export interface MenuProps
+  extends Omit<PopoverProps, 'active'>,
+    Omit<ComponentPropsWithoutRef<'div'>, 'onClick'> {
   /**
    * The menu items, or submenus
    * @type `<MenuItem />` | `<SubMenu />` | `<MenuGroup />` | `<MenuSeparator />`

packages/popover/src/Popover.types.ts

@@ -190,6 +190,7 @@ export type PopoverProps = {
   /**
    * Click event handler passed to the root div element within the portal container.
    */
+  // TODO: This should be typed with `HTMLDivElement`
   onClick?: React.MouseEventHandler;
 
   /**

packages/split-button/src/Menu/Menu.tsx

@@ -47,8 +47,8 @@ export const Menu = ({
 }: MenuProps) => {
   const { theme } = useDarkMode();
   const [uncontrolledOpen, uncontrolledSetOpen] = useState<boolean>(false);
-  const buttonRef = useRef<null | HTMLButtonElement>(null);
-  const menuRef = useRef<null | HTMLDivElement>(null);
+  const buttonRef = useRef<HTMLButtonElement>(null);
+  const menuRef = useRef<HTMLDivElement>(null);
 
   // TODO: make hook
   const setOpen =