fix(Menu): Focus on first menu item on click (#20955)

* fix(Menu): Focus on first menu item on click * Change files * use single effect * fix conditions
microsoft · Dec 8, 2021 · 6ca0355 · 6ca0355
1 parent a0b9643
commit 6ca0355
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 22 deletions.
diff --git a/change/@fluentui-react-menu-108a6441-1cfd-416b-bf2e-86d29fca5c90.json b/change/@fluentui-react-menu-108a6441-1cfd-416b-bf2e-86d29fca5c90.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "fix: Focus on first menu item on click",
+  "packageName": "@fluentui/react-menu",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-menu/e2e/Menu.e2e.ts b/packages/react-menu/e2e/Menu.e2e.ts
@@ -21,24 +21,13 @@ describe('Menu', () => {
   });
 
   describe('MenuTrigger', () => {
-    it('should open menu when clicked', () => {
-      cy.loadStory(menuStoriesTitle, defaultStory)
-        .get(menuTriggerSelector)
-        .click()
-        .get(menuSelector)
-        .should('be.visible');
-    });
-
-    it('should focus first menu item on down arrow when focus is on the trigger', () => {
+    it('should open menu and focus first item when clicked', () => {
       cy.loadStory(menuStoriesTitle, defaultStory)
         .get(menuTriggerSelector)
         .click()
         .get(menuSelector)
         .should('be.visible')
-        .get(menuTriggerSelector)
-        .type('{downarrow}')
         .get(menuItemSelector)
-        .first()
         .should('be.focused');
     });
 
@@ -260,7 +249,7 @@ describe('Menu', () => {
   // [nestedMenuStory, nestedMenuControlledStory].forEach(story => {
   [nestedMenuStory, nestedMenuControlledStory].forEach(story => {
     describe(`Nested Menus (${story.includes('Controlled') ? 'Controlled' : 'Uncontrolled'})`, () => {
-      it('should open on trigger hover', () => {
+      it('should open on trigger hover and focus first item', () => {
         cy.loadStory(menuStoriesTitle, story)
           .get(menuTriggerSelector)
           .click()
@@ -269,11 +258,16 @@ describe('Menu', () => {
             cy.get(menuTriggerSelector).trigger('mousemove');
           })
           .get(menuSelector)
-          .should('have.length', 2);
+          .should('have.length', 2)
+          .get(menuSelector)
+          .eq(1)
+          .within(() => {
+            cy.get(menuItemSelector).first().should('be.focused');
+          });
       });
 
       ['{rightarrow}', '{enter}', ' '].forEach(key => {
-        it(`should open on trigger ${key === ' ' ? 'space' : key}`, () => {
+        it(`should open on trigger ${key === ' ' ? 'space' : key} and focus first item`, () => {
           cy.loadStory(menuStoriesTitle, story)
             .get(menuTriggerSelector)
             .click()

diff --git a/packages/react-menu/src/components/Menu/useMenu.tsx b/packages/react-menu/src/components/Menu/useMenu.tsx
@@ -213,13 +213,17 @@ const useMenuOpenState = (
   }, [findPrevFocusable, state.triggerRef]);
 
   React.useEffect(() => {
-    if (!shouldHandleKeyboardRef.current) {
-      return;
+    if (open) {
+      focusFirst();
     }
+  }, [open, focusFirst]);
 
+  React.useEffect(() => {
     if (open) {
       focusFirst();
-    } else {
+    }
+
+    if (shouldHandleKeyboardRef.current && !open) {
       if (shouldHandleTabRef.current && !state.isSubmenu) {
         pressedShiftRef.current ? focusBeforeMenuTrigger() : focusAfterMenuTrigger();
       } else {

diff --git a/packages/react-menu/src/components/MenuTrigger/useTriggerElement.ts b/packages/react-menu/src/components/MenuTrigger/useTriggerElement.ts
@@ -81,10 +81,6 @@ export const useTriggerElement = (state: MenuTriggerState): MenuTriggerState =>
       focusFirst();
     }
 
-    if (open && key === ArrowDown && !isSubmenu) {
-      focusFirst();
-    }
-
     child?.props?.onKeyDown?.(e);
   });