diff --git a/packages/menu/src/MenuContainer.spec.tsx b/packages/menu/src/MenuContainer.spec.tsx index 0acea5a2c..427f195ba 100644 --- a/packages/menu/src/MenuContainer.spec.tsx +++ b/packages/menu/src/MenuContainer.spec.tsx @@ -241,16 +241,45 @@ describe('MenuContainer', () => { expect(menu).not.toBeVisible(); }); - it('closes menu on blur', async () => { + it('closes the menu on blur due to a body click, returns focus to the trigger the first time, and focuses the body on following clicks.', async () => { const { getByTestId } = render(); const trigger = getByTestId('trigger'); const menu = getByTestId('menu'); await waitFor(async () => { await user.click(trigger); + }); + expect(menu).toBeVisible(); + + await waitFor(async () => { + await user.click(document.body); + }); + expect(trigger).toHaveFocus(); + expect(menu).not.toBeVisible(); + + await waitFor(async () => { await user.click(document.body); }); + expect(document.body).toHaveFocus(); + }); + + it('closes menu on blur and moves focus to focusable element', async () => { + const { getByTestId } = render( + <> + + + + ); + const trigger = getByTestId('trigger'); + const menu = getByTestId('menu'); + const button = getByTestId('focusable'); + + await waitFor(async () => { + await user.click(trigger); + await user.click(button); + }); + expect(button).toHaveFocus(); expect(menu).not.toBeVisible(); }); @@ -887,7 +916,7 @@ describe('MenuContainer', () => { expect(fruit1).toHaveAttribute('aria-checked', 'true'); }); - it('returns normal keyboard navigation after menu closes', async () => { + it('returns focus to trigger before resuming normal tab navigation after menu closes', async () => { const { getByText, getByTestId } = render( <> @@ -895,13 +924,25 @@ describe('MenuContainer', () => { ); const trigger = getByTestId('trigger'); + const menu = getByTestId('menu'); const nextFocusedElement = getByText('focus me'); trigger.focus(); await waitFor(async () => { - await user.keyboard('{Enter}'); // select trigger - await user.keyboard('{Enter}'); // select first item + await user.keyboard('{ArrowDown}'); + }); + + expect(menu).toBeVisible(); + + await waitFor(async () => { + await user.keyboard('{Tab}'); + }); + + expect(menu).not.toBeVisible(); + expect(trigger).toHaveFocus(); + + await waitFor(async () => { await user.keyboard('{Tab}'); }); diff --git a/packages/menu/src/useMenu.ts b/packages/menu/src/useMenu.ts index 0ec4fa334..c52dad233 100644 --- a/packages/menu/src/useMenu.ts +++ b/packages/menu/src/useMenu.ts @@ -375,16 +375,52 @@ export const useMenu = { - const path = event.composedPath(); + /** + * 1. Determine if the next element receiving focus is focusable + * (event.relatedTarget is null when focus moves to non-focusable elements or body). + * + * 2. When an element loses focus (on blur), and focus moves to a non-focusable element + * like , `event.relatedTarget` should be `null`. However, due to a bug in jsdom + * (prior to version 24.1.2), `relatedTarget` is incorrectly set to the `Document` node + * (`nodeName === '#document'`). + * + * Currently, `jest-environment-jsdom` (v29.7.0) uses jsdom@20.0.3, which still has this issue. + * Until Jest updates its jsdom dependency, this workaround ensures accurate + * testing of focus behavior. + * + * @see https://github.com/jsdom/jsdom/pull/3767 + * @see https://github.com/jsdom/jsdom/releases/tag/24.1.2 + * @see https://github.com/jestjs/jest/blob/v29.7.0/packages/jest-environment-jsdom/package.json + * + * 3. Skip focus-return to trigger in these scenarios: + * a. Focus is moving to another focusable element + * b. Menu is closed and focus would naturally go to body + */ + const handleBlur = useCallback( + (event: React.FocusEvent) => { + const win = environment || window; - if (!path.includes(menuRef.current!) && !path.includes(triggerRef.current!)) { - returnFocusToTrigger(); - closeMenu(StateChangeTypes.MenuBlur); - } + setTimeout(() => { + // Timeout is required to ensure blur is handled after focus + const activeElement = win.document.activeElement; + const isMenuOrTriggerFocused = + menuRef.current?.contains(activeElement) || triggerRef.current?.contains(activeElement); + + if (!isMenuOrTriggerFocused) { + const nextElementIsFocusable = + !!event.relatedTarget /* [1] */ && + event.relatedTarget?.nodeName !== '#document'; /* [2] */ + + const shouldSkipFocusReturn = + nextElementIsFocusable || (!controlledIsExpanded && !nextElementIsFocusable); /* [3] */ + + returnFocusToTrigger(shouldSkipFocusReturn); + + closeMenu(StateChangeTypes.MenuBlur); + } + }); }, - [closeMenu, menuRef, returnFocusToTrigger, triggerRef] + [closeMenu, controlledIsExpanded, environment, menuRef, returnFocusToTrigger, triggerRef] ); const handleMenuMouseLeave = useCallback(() => { @@ -602,18 +638,15 @@ export const useMenu = { - win.document.removeEventListener('click', handleMenuBlur, true); win.document.removeEventListener('keydown', handleMenuKeyDown, true); }; - }, [controlledIsExpanded, handleMenuBlur, handleMenuKeyDown, environment]); + }, [controlledIsExpanded, handleMenuKeyDown, environment]); /** * When the menu is opened, this effect sets focus on the current menu item using `focusedValue` @@ -690,7 +723,15 @@ export const useMenu = ( - ({ onClick, onKeyDown, type = 'button', role = 'button', disabled, ...other } = {}) => ({ + ({ + onBlur, + onClick, + onKeyDown, + type = 'button', + role = 'button', + disabled, + ...other + } = {}) => ({ ...other, 'data-garden-container-id': 'containers.menu.trigger', 'data-garden-container-version': PACKAGE_VERSION, @@ -702,14 +743,22 @@ export const useMenu = ( - ({ role = 'menu', onMouseLeave, ...other } = {}) => ({ + ({ role = 'menu', onBlur, onMouseLeave, ...other } = {}) => ({ ...other, ...getGroupProps({ onMouseLeave: composeEventHandlers(onMouseLeave, handleMenuMouseLeave) @@ -719,9 +768,10 @@ export const useMenu = (