Skip to content

Commit 8b0e7d7

Browse files
authored
fix: Make submenu hover focus remain on the trigger and collapse submenus when hovering root menu (#8666)
* fix: hovering submenutrigger should keep focus on the trigger also makes it so ArrowRight still moves to the submenu after hovering, not sure if this breaks VO moving focus to the dismiss button though... * make submenu close if any other item in the same menu is focused this fixes the case where the user hovers a root menu level item when there are multiple submenu levels open but only the last one closes
1 parent 2ee02a0 commit 8b0e7d7

File tree

4 files changed

+50
-9
lines changed

4 files changed

+50
-9
lines changed

packages/@react-aria/menu/src/useSubmenuTrigger.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {AriaMenuItemProps} from './useMenuItem';
1414
import {AriaMenuOptions} from './useMenu';
1515
import type {AriaPopoverProps, OverlayProps} from '@react-aria/overlays';
1616
import {FocusableElement, FocusStrategy, KeyboardEvent, Node, PressEvent, RefObject} from '@react-types/shared';
17-
import {focusWithoutScrolling, useEffectEvent, useId, useLayoutEffect} from '@react-aria/utils';
17+
import {focusWithoutScrolling, useEffectEvent, useEvent, useId, useLayoutEffect} from '@react-aria/utils';
1818
import type {SubmenuTriggerState} from '@react-stately/menu';
1919
import {useCallback, useRef} from 'react';
2020
import {useLocale} from '@react-aria/i18n';
@@ -223,11 +223,13 @@ export function useSubmenuTrigger<T>(props: AriaSubmenuTriggerProps, state: Subm
223223
}
224224
};
225225

226-
let onBlur = (e) => {
227-
if (state.isOpen && (parentMenuRef.current?.contains(e.relatedTarget))) {
226+
useEvent(parentMenuRef, 'focusin', (e) => {
227+
// If we detect focus moved to a different item in the same menu that the currently open submenu trigger is in
228+
// then close the submenu. This is for a case where the user hovers a root menu item when multiple submenus are open
229+
if (state.isOpen && (parentMenuRef.current?.contains(e.target as HTMLElement) && e.target !== ref.current)) {
228230
onSubmenuClose();
229231
}
230-
};
232+
});
231233

232234
let shouldCloseOnInteractOutside = (target) => {
233235
if (target !== ref.current) {
@@ -249,7 +251,6 @@ export function useSubmenuTrigger<T>(props: AriaSubmenuTriggerProps, state: Subm
249251
onPress,
250252
onHoverChange,
251253
onKeyDown: submenuTriggerKeyDown,
252-
onBlur,
253254
isOpen: state.isOpen
254255
},
255256
submenuProps,

packages/react-aria-components/src/Menu.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,12 @@ export const SubmenuTrigger = /*#__PURE__*/ createBranchComponent('submenutrigg
131131
<Provider
132132
values={[
133133
[MenuItemContext, {...submenuTriggerProps, onAction: undefined, ref: itemRef}],
134-
[MenuContext, submenuProps],
134+
[MenuContext, {
135+
ref: submenuRef,
136+
...submenuProps
137+
}],
135138
[OverlayTriggerStateContext, submenuTriggerState],
136139
[PopoverContext, {
137-
ref: submenuRef,
138140
trigger: 'SubmenuTrigger',
139141
triggerRef: itemRef,
140142
placement: 'end top',

packages/react-aria-components/src/Popover.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,11 +190,12 @@ function PopoverInner({state, isExiting, UNSTABLE_portalContainer, clearContexts
190190
}, [ref, shouldBeDialog]);
191191

192192
// Focus the popover itself on mount, unless a child element is already focused.
193+
// Skip this for submenus since hovering a submenutrigger should keep focus on the trigger
193194
useEffect(() => {
194-
if (isDialog && ref.current && !ref.current.contains(document.activeElement)) {
195+
if (isDialog && props.trigger !== 'SubmenuTrigger' && ref.current && !ref.current.contains(document.activeElement)) {
195196
focusSafely(ref.current);
196197
}
197-
}, [isDialog, ref]);
198+
}, [isDialog, ref, props.trigger]);
198199

199200
let children = useMemo(() => {
200201
let children = renderProps.children;

packages/react-aria-components/test/AriaMenu.test-util.tsx

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,43 @@ export const AriaMenuTests = ({renderers, setup, prefix}: AriaMenuTestProps): vo
693693
expect(nestedSubmenu).not.toBeInTheDocument();
694694
expect(document.activeElement).toBe(nestedSubmenuTrigger);
695695
});
696+
697+
it('should close the submenu if another item in the same menu is focused', async () => {
698+
let tree = (renderers.submenus!)();
699+
let menuTester = testUtilUser.createTester('Menu', {user, root: tree.container});
700+
await menuTester.open();
701+
let menu = menuTester.menu;
702+
let submenuTrigger = menuTester.submenuTriggers[0];
703+
let submenuUtil = (await menuTester.openSubmenu({submenuTrigger}))!;
704+
act(() => {jest.runAllTimers();});
705+
let submenu = submenuUtil.menu;
706+
expect(submenu).toBeInTheDocument();
707+
let nestedSubmenuTrigger = submenuUtil.submenuTriggers[0];
708+
let nestedSubmenuUtil = (await submenuUtil.openSubmenu({submenuTrigger: nestedSubmenuTrigger}))!;
709+
act(() => {jest.runAllTimers();});
710+
let nestedSubmenu = nestedSubmenuUtil.menu;
711+
expect(submenu).toBeInTheDocument();
712+
await user.hover(menuTester.options()[0]);
713+
act(() => {jest.runAllTimers();});
714+
expect(nestedSubmenu).not.toBeInTheDocument();
715+
expect(submenu).not.toBeInTheDocument();
716+
expect(menu).toBeInTheDocument();
717+
});
718+
719+
it('should retain focus on the submenu trigger when hovering it', async () => {
720+
let tree = (renderers.submenus!)();
721+
let menuTester = testUtilUser.createTester('Menu', {user, root: tree.container});
722+
await menuTester.open();
723+
await user.hover(menuTester.submenuTriggers[0]);
724+
act(() => {jest.runAllTimers();});
725+
expect(menuTester.submenuTriggers[0]).toHaveAttribute('aria-expanded', 'true');
726+
expect(document.activeElement).toBe(menuTester.submenuTriggers[0]);
727+
728+
// It should also allow the user to move focus into the submenu via ArrowRight
729+
await user.keyboard('{ArrowRight}');
730+
let submenu = tree.getAllByRole('menu')[1];
731+
expect(document.activeElement).toBe(within(submenu).getAllByRole('menuitem')[0]);
732+
});
696733
});
697734
}
698735

0 commit comments

Comments
 (0)