diff --git a/change/@fluentui-react-menu-9787aa9c-cdf8-41b8-a9d6-c8d7e1f3c502.json b/change/@fluentui-react-menu-9787aa9c-cdf8-41b8-a9d6-c8d7e1f3c502.json new file mode 100644 index 00000000000000..bd32cff23f6a12 --- /dev/null +++ b/change/@fluentui-react-menu-9787aa9c-cdf8-41b8-a9d6-c8d7e1f3c502.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: Tab should focus the window if trigger is the last focusable element", + "packageName": "@fluentui/react-menu", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} 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 index 916adec9696e60..4f438e161c5cbf 100644 --- a/packages/react-components/react-menu/src/components/Menu/Menu.cy.tsx +++ b/packages/react-components/react-menu/src/components/Menu/Menu.cy.tsx @@ -931,9 +931,29 @@ describe(`Nested Menus`, () => { .get(menuSelector) .eq(1) .realPress('Tab'); + cy.contains('After').should('be.focused').get(menuSelector).should('not.exist'); }); + it('should move focus out of document if menu trigger is the last focusable element in DOM', () => { + mount( + , + ); + + cy.get(menuTriggerSelector) + .click() + .get(menuSelector) + .within(() => { + cy.get(menuItemSelector).eq(4).type('{rightarrow}'); + }) + .get(menuSelector) + .eq(1) + .realPress('Tab'); + + cy.realPress(['Shift', 'Tab']); + cy.get(menuTriggerSelector).should('be.focused'); + }); + it('should be able to shift tab to previous element after the root trigger', () => { mount( <> diff --git a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx index 542bdaca534d11..a8492c9de0d054 100644 --- a/packages/react-components/react-menu/src/components/Menu/useMenu.tsx +++ b/packages/react-components/react-menu/src/components/Menu/useMenu.tsx @@ -14,7 +14,6 @@ import { useMenuContext_unstable } from '../../contexts/menuContext'; import { MENU_ENTER_EVENT, useOnMenuMouseEnter } from '../../utils/index'; import { useIsSubmenu } from '../../utils/useIsSubmenu'; import type { MenuOpenChangeData, MenuOpenEvent, MenuProps, MenuState } from './Menu.types'; -import { Tab } from '@fluentui/keyboard-keys'; /** * Create the state required to render Menu. @@ -159,8 +158,6 @@ const useMenuOpenState = ( const onOpenChange: MenuProps['onOpenChange'] = useEventCallback((e, data) => state.onOpenChange?.(e, data)); const shouldHandleCloseRef = React.useRef(false); - const shouldHandleTabRef = React.useRef(false); - const pressedShiftRef = React.useRef(false); const setOpenTimeout = React.useRef(0); const enteringTriggerRef = React.useRef(false); @@ -182,13 +179,6 @@ const useMenuOpenState = ( shouldHandleCloseRef.current = true; } - if (e.type === 'keydown') { - if ((e as React.KeyboardEvent).key === Tab) { - shouldHandleTabRef.current = true; - pressedShiftRef.current = (e as React.KeyboardEvent).shiftKey; - } - } - if (data.bubble) { parentSetOpen(e, { ...data }); } @@ -261,39 +251,29 @@ const useMenuOpenState = ( }, []); // Manage focus for open state - const { findFirstFocusable, findNextFocusable, findPrevFocusable } = useFocusFinders(); + const { findFirstFocusable } = useFocusFinders(); const focusFirst = React.useCallback(() => { const firstFocusable = findFirstFocusable(state.menuPopoverRef.current); firstFocusable?.focus(); }, [findFirstFocusable, state.menuPopoverRef]); - const focusAfterMenuTrigger = React.useCallback(() => { - const nextFocusable = findNextFocusable(state.triggerRef.current); - nextFocusable?.focus(); - }, [findNextFocusable, state.triggerRef]); - - const focusBeforeMenuTrigger = React.useCallback(() => { - const prevFocusable = findPrevFocusable(state.triggerRef.current); - prevFocusable?.focus(); - }, [findPrevFocusable, state.triggerRef]); - React.useEffect(() => { if (open) { focusFirst(); } else { if (shouldHandleCloseRef.current) { - if (shouldHandleTabRef.current && !state.isSubmenu) { - pressedShiftRef.current ? focusBeforeMenuTrigger() : focusAfterMenuTrigger(); - } else { - state.triggerRef.current?.focus(); - } + // We know that React effects are sync so we focus the trigger here + // after any event handler (event handlers will update state and re-render). + // Since the browser only performs the default behaviour for the Tab key once + // keyboard events have fully bubbled up the window, the browser will move + // focus to the next tabbable element before/after the trigger if needed. + // If the Tab key was not pressed, focus will remain on the trigger as expected. + state.triggerRef.current?.focus(); } } shouldHandleCloseRef.current = false; - shouldHandleTabRef.current = false; - pressedShiftRef.current = false; - }, [state.triggerRef, state.isSubmenu, open, focusFirst, focusAfterMenuTrigger, focusBeforeMenuTrigger]); + }, [state.triggerRef, state.isSubmenu, open, focusFirst]); return [open, setOpen] as const; }; diff --git a/packages/react-components/react-menu/src/components/MenuPopover/useMenuPopover.ts b/packages/react-components/react-menu/src/components/MenuPopover/useMenuPopover.ts index 6f8b02bc52ebd7..dbc2a5ab1ebbea 100644 --- a/packages/react-components/react-menu/src/components/MenuPopover/useMenuPopover.ts +++ b/packages/react-components/react-menu/src/components/MenuPopover/useMenuPopover.ts @@ -85,7 +85,6 @@ export const useMenuPopover_unstable = (props: MenuPopoverProps, ref: React.Ref< if (key === Tab) { setOpen(event, { open: false, keyboard: true, type: 'menuPopoverKeyDown', event }); - event.preventDefault(); } onKeyDownOriginal?.(event);