fix: Menu should not steal focus on re-render

Follow up from microsoft#27414. The focusing effect should not re-run if the value of `firstMount` changes.
ling1726 · Apr 25, 2023 · 5802e6e · 5802e6e
1 parent 948b5bf
commit 5802e6e
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 2 deletions.
diff --git a/packages/react-components/react-menu/src/components/Menu/Menu.cy.tsx b/packages/react-components/react-menu/src/components/Menu/Menu.cy.tsx
@@ -132,6 +132,41 @@ describe('MenuTrigger', () => {
     );
     cy.get(menuTriggerSelector).should('not.be.focused');
   });
+
+  it('should not focus itself after re-render', () => {
+    const Example = () => {
+      const [count, setCount] = React.useState(0);
+
+      React.useEffect(() => {
+        // trigger 2 re-renders to make sure that the focus effect has finished running
+        // after first re-render
+        if (count < 2) {
+          setCount(c => c + 1);
+        }
+      }, [count]);
+
+      return (
+        <>
+          <Menu>
+            <MenuTrigger disableButtonEnhancement>
+              <button data-status={count < 2 ? 'incomplete' : 'complete'} id={menuTriggerId}>
+                Menu
+              </button>
+            </MenuTrigger>
+            <MenuPopover>
+              <MenuList>
+                <MenuItem>Item</MenuItem>
+              </MenuList>
+            </MenuPopover>
+          </Menu>
+          <input type="text" />
+        </>
+      );
+    };
+
+    mount(<Example />);
+    cy.get(menuTriggerSelector).should('have.attr', 'data-status', 'complete').should('not.be.focused');
+  });
 });
 
 describe('Custom Trigger', () => {
@@ -391,7 +426,7 @@ describe('MenuItemRadio', () => {
 });
 
 describe('Menu', () => {
-  it('should not focus trigger on dismiss if another elemnt is focused', () => {
+  it('should not focus trigger on dismiss if another element is focused', () => {
     mount(
       <>
         <Menu>

diff --git a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx
@@ -292,7 +292,9 @@ const useMenuOpenState = (
         }
       }
     }
-  }, [state.triggerRef, state.isSubmenu, open, focusFirst, firstMount, targetDocument, state.menuPopoverRef]);
+    // firstMount change should not re-run this effect
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [state.triggerRef, state.isSubmenu, open, focusFirst, targetDocument, state.menuPopoverRef]);
 
   return [open, setOpen] as const;
 };