fix: Menu trigger should be focused when dismissed on menuitem click (#…

…25289) * fix: Menu trigger should be focused when dismissed on menuitem click Replaces `shouldHandleKeyboardRef` with `shouldHandlCloseRef` so that mouse and keyboard click are handled the same way. * changefiles * revert overflow changes * update change file
microsoft · Oct 19, 2022 · 099d9f0 · 099d9f0
1 parent bd212e7
commit 099d9f0
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 17 deletions.
diff --git a/change/@fluentui-react-menu-2426a6a3-09c5-490e-95d8-8ca4b9c9950e.json b/change/@fluentui-react-menu-2426a6a3-09c5-490e-95d8-8ca4b9c9950e.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix: Menu trigger should be focused when dismissed by menuitem click",
+  "packageName": "@fluentui/react-menu",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/change/@fluentui-react-overflow-e0db4922-5cc1-48d6-a1ee-389ba71df899.json b/change/@fluentui-react-overflow-e0db4922-5cc1-48d6-a1ee-389ba71df899.json
@@ -0,0 +1,7 @@
+{
+  "type": "none",
+  "comment": "Remove it.only from unit tests",
+  "packageName": "@fluentui/react-overflow",
+  "email": "[email protected]",
+  "dependentChangeType": "none"
+}
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
@@ -551,6 +551,26 @@ describe('Menu', () => {
     cy.get(menuTriggerSelector).click().get(menuSelector).should('exist').realPress(['Shift', 'Tab']);
     cy.contains('Before').should('be.focused').get(menuSelector).should('not.exist');
   });
+
+  (['Enter', 'Space'] as const).forEach(key => {
+    it(`should close menu and focus trigger on ${key}`, () => {
+      mount(
+        <Menu>
+          <MenuTrigger>
+            <button>Menu</button>
+          </MenuTrigger>
+          <MenuPopover>
+            <MenuList>
+              <MenuItem>Item</MenuItem>
+            </MenuList>
+          </MenuPopover>
+        </Menu>,
+      );
+
+      cy.get(menuTriggerSelector).click().get(menuSelector).should('exist').realPress(key);
+      cy.get(menuSelector).should('not.exist').get(menuTriggerSelector).should('be.focused');
+    });
+  });
 });
 
 describe('SplitMenuItem', () => {

diff --git a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx
@@ -162,7 +162,7 @@ const useMenuOpenState = (
   const parentSetOpen = useMenuContext_unstable(context => context.setOpen);
   const onOpenChange: MenuState['onOpenChange'] = useEventCallback((e, data) => state.onOpenChange?.(e, data));
 
-  const shouldHandleKeyboardRef = React.useRef(false);
+  const shouldHandleCloseRef = React.useRef(false);
   const shouldHandleTabRef = React.useRef(false);
   const pressedShiftRef = React.useRef(false);
   const setOpenTimeout = React.useRef(0);
@@ -183,10 +183,10 @@ const useMenuOpenState = (
 
     if (!data.open) {
       state.setContextTarget(undefined);
+      shouldHandleCloseRef.current = true;
     }
 
     if (e.type === 'keydown') {
-      shouldHandleKeyboardRef.current = true;
       if ((e as React.KeyboardEvent<HTMLElement>).key === Tab) {
         shouldHandleTabRef.current = true;
         pressedShiftRef.current = (e as React.KeyboardEvent<HTMLElement>).shiftKey;
@@ -284,26 +284,20 @@ const useMenuOpenState = (
   React.useEffect(() => {
     if (open) {
       focusFirst();
-    }
-  }, [open, focusFirst]);
-
-  React.useEffect(() => {
-    if (open) {
-      focusFirst();
-    }
-
-    if (shouldHandleKeyboardRef.current && !open) {
-      if (shouldHandleTabRef.current && !state.isSubmenu) {
-        pressedShiftRef.current ? focusBeforeMenuTrigger() : focusAfterMenuTrigger();
-      } else {
-        state.triggerRef.current?.focus();
+    } else {
+      if (shouldHandleCloseRef.current) {
+        if (shouldHandleTabRef.current && !state.isSubmenu) {
+          pressedShiftRef.current ? focusBeforeMenuTrigger() : focusAfterMenuTrigger();
+        } else {
+          state.triggerRef.current?.focus();
+        }
       }
     }
 
-    shouldHandleKeyboardRef.current = false;
+    shouldHandleCloseRef.current = false;
     shouldHandleTabRef.current = false;
     pressedShiftRef.current = false;
   }, [state.triggerRef, state.isSubmenu, open, focusFirst, focusAfterMenuTrigger, focusBeforeMenuTrigger]);
 
-  return [open ?? false, setOpen] as const;
+  return [open, setOpen] as const;
 };