Skip to content

Commit

Permalink
fix: Menu triggers no longer take focus when they are mounted (#25016)
Browse files Browse the repository at this point in the history
This is likely a regression from #24738. Specifically, the removal of `shouldHandleKeyboardRef` from useMenu.tsx is causing it to always call `state.triggerRef.current?.focus()` when mounted.
  • Loading branch information
behowell authored Oct 3, 2022
1 parent 7308dce commit b0b7f99
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: Menu triggers no longer take focus when they are mounted",
"packageName": "@fluentui/react-menu",
"email": "behowell@microsoft.com",
"dependentChangeType": "patch"
}
16 changes: 16 additions & 0 deletions packages/react-components/react-menu/e2e/Menu.e2e.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,22 @@ describe('MenuTrigger', () => {
cy.get(menuSelector).should('be.visible').get(menuItemSelector).first().should('be.focused');
});
});

it('should not automatically focus itself when mounted', () => {
mount(
<Menu>
<MenuTrigger>
<button>Menu</button>
</MenuTrigger>
<MenuPopover>
<MenuList>
<MenuItem>Item</MenuItem>
</MenuList>
</MenuPopover>
</Menu>,
);
cy.get(menuTriggerSelector).should('not.be.focused');
});
});

describe('Custom Trigger', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +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 shouldHandleTabRef = React.useRef(false);
const pressedShiftRef = React.useRef(false);
const setOpenTimeout = React.useRef(0);
Expand All @@ -185,6 +186,7 @@ const useMenuOpenState = (
}

if (e.type === 'keydown' && (e as React.KeyboardEvent<HTMLElement>).key === Tab) {
shouldHandleKeyboardRef.current = true;
shouldHandleTabRef.current = true;
pressedShiftRef.current = (e as React.KeyboardEvent<HTMLElement>).shiftKey;
}
Expand Down Expand Up @@ -288,14 +290,15 @@ const useMenuOpenState = (
focusFirst();
}

if (!open) {
if (shouldHandleKeyboardRef.current && !open) {
if (shouldHandleTabRef.current && !state.isSubmenu) {
pressedShiftRef.current ? focusBeforeMenuTrigger() : focusAfterMenuTrigger();
} else {
state.triggerRef.current?.focus();
}
}

shouldHandleKeyboardRef.current = false;
shouldHandleTabRef.current = false;
pressedShiftRef.current = false;
}, [state.triggerRef, state.isSubmenu, open, focusFirst, focusAfterMenuTrigger, focusBeforeMenuTrigger]);
Expand Down

0 comments on commit b0b7f99

Please sign in to comment.