fix(unstable_Menu): close menu after clicking inside Popup/Modal (#2504)

Author: amje

src/components/lab/Menu/Menu.tsx

@@ -44,13 +44,15 @@ const b = block('lab-menu');
 function MenuPopupContent({
     open,
     onRequestClose,
+    isNested,
     children,
     className,
     style,
     qa,
 }: Pick<MenuProps, 'children' | 'className' | 'style' | 'qa'> & {
     open: boolean;
     onRequestClose: () => void;
+    isNested: boolean;
 }) {
     const tree = useFloatingTree();
     const nodeId = useFloatingParentNodeId();
@@ -61,7 +63,7 @@ function MenuPopupContent({
 
         function handleTreeClick() {
             // Closing only the root Menu so the closing animation runs once for all menus due to shared portal container
-            if (!parentId) {
+            if (!isNested) {
                 onRequestClose();
             }
         }
@@ -80,7 +82,7 @@ function MenuPopupContent({
             tree.events.off('click', handleTreeClick);
             tree.events.off('menuopen', handleSubMenuOpen);
         };
-    }, [onRequestClose, tree, nodeId, parentId]);
+    }, [onRequestClose, tree, nodeId, parentId, isNested]);
 
     React.useEffect(() => {
         if (open && tree) {
@@ -296,6 +298,7 @@ export function Menu({
                         <MenuPopupContent
                             open={isOpen}
                             onRequestClose={handleContentRequestClose}
+                            isNested={isNested}
                             className={className}
                             style={style}
                             qa={qa}

src/components/lab/Menu/__tests__/Menu.test.tsx

@@ -1,24 +1,26 @@
 import userEvent from '@testing-library/user-event';
 
 import {act, getAllByRole, render, screen, waitFor} from '../../../../../test-utils/utils';
+import {Modal} from '../../../Modal';
+import {Popup} from '../../../Popup';
 import {Menu} from '../Menu';
 
 const TRIGGER_QA = 'trigger';
 const MENU_QA = 'menu';
 const SUBMENU_QA = 'submenu';
 
-function renderSimpleMenu() {
-    return render(
+function SimpleMenu() {
+    return (
         <Menu trigger={<Menu.Trigger qa={TRIGGER_QA} />} qa={MENU_QA}>
             <Menu.Item>Item 1</Menu.Item>
             <Menu.Item>Item 2</Menu.Item>
             <Menu.Item>Item 3</Menu.Item>
-        </Menu>,
+        </Menu>
     );
 }
 
-function renderComplexMenu() {
-    return render(
+function ComplexMenu() {
+    return (
         <Menu trigger={<Menu.Trigger qa={TRIGGER_QA} />} qa={MENU_QA}>
             <Menu.Item>Item 1</Menu.Item>
             <Menu.Item>Item 2</Menu.Item>
@@ -29,20 +31,20 @@ function renderComplexMenu() {
                     <Menu.Item>Item 4</Menu.Item>
                 </Menu>
             </Menu.Item>
-        </Menu>,
+        </Menu>
     );
 }
 
 describe('Menu', () => {
     test('should render default trigger', () => {
-        renderSimpleMenu();
+        render(<SimpleMenu />);
         const trigger = screen.getByTestId(TRIGGER_QA);
 
         expect(trigger).toBeVisible();
     });
 
     test('should open menu by click', async () => {
-        renderSimpleMenu();
+        render(<SimpleMenu />);
         const user = userEvent.setup();
         const trigger = screen.getByTestId(TRIGGER_QA);
 
@@ -57,7 +59,7 @@ describe('Menu', () => {
     test.each(['{Space}', '{Enter}', '{ArrowDown}', '{ArrowUp}'])(
         'should open menu by keyboard %s',
         async (key) => {
-            renderSimpleMenu();
+            render(<SimpleMenu />);
             const user = userEvent.setup();
             const trigger = screen.getByTestId(TRIGGER_QA);
 
@@ -80,7 +82,55 @@ describe('Menu', () => {
     );
 
     test('should close menu by selecting an item', async () => {
-        renderSimpleMenu();
+        render(<SimpleMenu />);
+        const user = userEvent.setup();
+        const trigger = screen.getByTestId(TRIGGER_QA);
+
+        expect(screen.queryByTestId(MENU_QA)).not.toBeInTheDocument();
+
+        await user.click(trigger);
+        await waitFor(() => {
+            expect(screen.getByTestId(MENU_QA)).toBeVisible();
+        });
+
+        const items = screen.getAllByRole('menuitem');
+
+        await user.click(items[1]);
+        await waitFor(() => {
+            expect(screen.queryByTestId(MENU_QA)).not.toBeInTheDocument();
+        });
+    });
+
+    test('should close menu by selecting an item inside Popup', async () => {
+        render(
+            <Popup open>
+                <SimpleMenu />
+            </Popup>,
+        );
+        const user = userEvent.setup();
+        const trigger = screen.getByTestId(TRIGGER_QA);
+
+        expect(screen.queryByTestId(MENU_QA)).not.toBeInTheDocument();
+
+        await user.click(trigger);
+        await waitFor(() => {
+            expect(screen.getByTestId(MENU_QA)).toBeVisible();
+        });
+
+        const items = screen.getAllByRole('menuitem');
+
+        await user.click(items[1]);
+        await waitFor(() => {
+            expect(screen.queryByTestId(MENU_QA)).not.toBeInTheDocument();
+        });
+    });
+
+    test('should close menu by selecting an item inside Modal', async () => {
+        render(
+            <Modal open>
+                <SimpleMenu />
+            </Modal>,
+        );
         const user = userEvent.setup();
         const trigger = screen.getByTestId(TRIGGER_QA);
 
@@ -100,7 +150,7 @@ describe('Menu', () => {
     });
 
     test('should have correct keyboard navigation', async () => {
-        renderSimpleMenu();
+        render(<SimpleMenu />);
         const user = userEvent.setup();
         const trigger = screen.getByTestId(TRIGGER_QA);
 
@@ -130,7 +180,7 @@ describe('Menu', () => {
     });
 
     test('should open submenu by hover', async () => {
-        renderComplexMenu();
+        render(<ComplexMenu />);
         const user = userEvent.setup();
         const trigger = screen.getByTestId(TRIGGER_QA);
 
@@ -152,7 +202,7 @@ describe('Menu', () => {
     });
 
     test('should open/close submenu by keyboard', async () => {
-        renderComplexMenu();
+        render(<ComplexMenu />);
         const user = userEvent.setup();
         const trigger = screen.getByTestId(TRIGGER_QA);
 
@@ -186,4 +236,32 @@ describe('Menu', () => {
         });
         expect(items[2]).toHaveClass(/active/);
     });
+
+    test('should not open by hover inside Popup', async () => {
+        render(
+            <Popup open>
+                <SimpleMenu />
+            </Popup>,
+        );
+        const user = userEvent.setup();
+        const trigger = screen.getByTestId(TRIGGER_QA);
+
+        await user.hover(trigger);
+
+        expect(screen.queryByTestId(MENU_QA)).not.toBeInTheDocument();
+    });
+
+    test('should not open by hover inside Modal', async () => {
+        render(
+            <Modal open>
+                <SimpleMenu />
+            </Modal>,
+        );
+        const user = userEvent.setup();
+        const trigger = screen.getByTestId(TRIGGER_QA);
+
+        await user.hover(trigger);
+
+        expect(screen.queryByTestId(MENU_QA)).not.toBeInTheDocument();
+    });
 });