Skip to content

Commit 3e383dd

Browse files
authored
[navigation menu] Ensure submenu triggers participate in composite list (#3344)
1 parent 3a02164 commit 3e383dd

File tree

3 files changed

+208
-15
lines changed

3 files changed

+208
-15
lines changed

packages/react/src/navigation-menu/list/NavigationMenuList.tsx

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { EMPTY_OBJECT } from '../../utils/constants';
99
import { NAVIGATION_MENU_TRIGGER_IDENTIFIER } from '../utils/constants';
1010
import { NavigationMenuDismissContext } from './NavigationMenuDismissContext';
1111
import { getEmptyRootContext } from '../../floating-ui-react/utils/getEmptyRootContext';
12+
import { useRenderElement } from '../../utils/useRenderElement';
1213

1314
/**
1415
* Contains a list of navigation menu items.
@@ -22,7 +23,7 @@ export const NavigationMenuList = React.forwardRef(function NavigationMenuList(
2223
) {
2324
const { className, render, ...elementProps } = componentProps;
2425

25-
const { orientation, open, floatingRootContext, positionerElement, value } =
26+
const { orientation, open, floatingRootContext, positionerElement, value, nested } =
2627
useNavigationMenuRootContext();
2728

2829
const fallbackContext = React.useMemo(() => getEmptyRootContext(), []);
@@ -50,20 +51,43 @@ export const NavigationMenuList = React.forwardRef(function NavigationMenuList(
5051
[open],
5152
);
5253

53-
const defaultProps: HTMLProps = {
54-
// `stopEventPropagation` won't stop the propagation if the end of the list is reached,
55-
// but we want to block it in this case.
56-
onKeyDown(event: React.KeyboardEvent<HTMLDivElement>) {
57-
const shouldStop =
58-
(orientation === 'horizontal' &&
59-
(event.key === 'ArrowLeft' || event.key === 'ArrowRight')) ||
60-
(orientation === 'vertical' && (event.key === 'ArrowUp' || event.key === 'ArrowDown'));
54+
// `stopEventPropagation` won't stop the propagation if the end of the list is reached,
55+
// but we want to block it in this case.
56+
// When nested, skip this handler so arrow keys can reach the parent CompositeRoot.
57+
const defaultProps: HTMLProps = nested
58+
? {}
59+
: {
60+
onKeyDown(event: React.KeyboardEvent<HTMLDivElement>) {
61+
const shouldStop =
62+
(orientation === 'horizontal' &&
63+
(event.key === 'ArrowLeft' || event.key === 'ArrowRight')) ||
64+
(orientation === 'vertical' && (event.key === 'ArrowUp' || event.key === 'ArrowDown'));
6165

62-
if (shouldStop) {
63-
event.stopPropagation();
64-
}
65-
},
66-
};
66+
if (shouldStop) {
67+
event.stopPropagation();
68+
}
69+
},
70+
};
71+
72+
const props = [dismissProps?.floating || EMPTY_OBJECT, defaultProps, elementProps];
73+
74+
// When nested, skip the CompositeRoot wrapper so that triggers can participate
75+
// in the parent Content's composite navigation context. Also skip the onKeyDown
76+
// handler that blocks propagation so arrow keys can reach the parent CompositeRoot.
77+
const element = useRenderElement('ul', componentProps, {
78+
state,
79+
ref: forwardedRef,
80+
props,
81+
enabled: nested,
82+
});
83+
84+
if (nested) {
85+
return (
86+
<NavigationMenuDismissContext.Provider value={dismissProps}>
87+
{element}
88+
</NavigationMenuDismissContext.Provider>
89+
);
90+
}
6791

6892
return (
6993
<NavigationMenuDismissContext.Provider value={dismissProps}>
@@ -72,7 +96,7 @@ export const NavigationMenuList = React.forwardRef(function NavigationMenuList(
7296
className={className}
7397
state={state}
7498
refs={[forwardedRef]}
75-
props={[dismissProps?.floating || EMPTY_OBJECT, defaultProps, elementProps]}
99+
props={props}
76100
loopFocus={false}
77101
orientation={orientation}
78102
tag="ul"

packages/react/src/navigation-menu/root/NavigationMenuRoot.test.tsx

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,82 @@ function TestInlineNestedNavigationMenu() {
143143
);
144144
}
145145

146+
function TestDeeplyNestedNavigationMenu() {
147+
return (
148+
<NavigationMenu.Root>
149+
<NavigationMenu.List>
150+
<NavigationMenu.Item value="item-1">
151+
<NavigationMenu.Trigger data-testid="trigger-1">Item 1</NavigationMenu.Trigger>
152+
153+
<NavigationMenu.Content data-testid="content-1">
154+
<NavigationMenu.Link href="#link-1" data-testid="link-1">
155+
Link 1
156+
</NavigationMenu.Link>
157+
{/* Level 2 */}
158+
<NavigationMenu.Root defaultValue="level2-item-1">
159+
<NavigationMenu.List>
160+
<NavigationMenu.Item value="level2-item-1">
161+
<NavigationMenu.Trigger data-testid="level2-trigger-1">
162+
Level 2 Item 1
163+
</NavigationMenu.Trigger>
164+
<NavigationMenu.Content data-testid="level2-content-1">
165+
<NavigationMenu.Link href="#level2-link-1" data-testid="level2-link-1">
166+
Level 2 Link 1
167+
</NavigationMenu.Link>
168+
{/* Level 3 */}
169+
<NavigationMenu.Root defaultValue="level3-item-1">
170+
<NavigationMenu.List>
171+
<NavigationMenu.Item value="level3-item-1">
172+
<NavigationMenu.Trigger data-testid="level3-trigger-1">
173+
Level 3 Item 1
174+
</NavigationMenu.Trigger>
175+
<NavigationMenu.Content data-testid="level3-content-1">
176+
<NavigationMenu.Link href="#level3-link-1">
177+
Level 3 Link 1
178+
</NavigationMenu.Link>
179+
</NavigationMenu.Content>
180+
</NavigationMenu.Item>
181+
<NavigationMenu.Item value="level3-item-2">
182+
<NavigationMenu.Trigger data-testid="level3-trigger-2">
183+
Level 3 Item 2
184+
</NavigationMenu.Trigger>
185+
<NavigationMenu.Content data-testid="level3-content-2">
186+
<NavigationMenu.Link href="#level3-link-2">
187+
Level 3 Link 2
188+
</NavigationMenu.Link>
189+
</NavigationMenu.Content>
190+
</NavigationMenu.Item>
191+
</NavigationMenu.List>
192+
<NavigationMenu.Viewport />
193+
</NavigationMenu.Root>
194+
</NavigationMenu.Content>
195+
</NavigationMenu.Item>
196+
<NavigationMenu.Item value="level2-item-2">
197+
<NavigationMenu.Trigger data-testid="level2-trigger-2">
198+
Level 2 Item 2
199+
</NavigationMenu.Trigger>
200+
<NavigationMenu.Content data-testid="level2-content-2">
201+
<NavigationMenu.Link href="#level2-link-2">Level 2 Link 2</NavigationMenu.Link>
202+
</NavigationMenu.Content>
203+
</NavigationMenu.Item>
204+
</NavigationMenu.List>
205+
<NavigationMenu.Viewport />
206+
</NavigationMenu.Root>
207+
</NavigationMenu.Content>
208+
</NavigationMenu.Item>
209+
</NavigationMenu.List>
210+
211+
<NavigationMenu.Portal>
212+
<NavigationMenu.Positioner>
213+
<NavigationMenu.Popup>
214+
<NavigationMenu.Viewport />
215+
</NavigationMenu.Popup>
216+
</NavigationMenu.Positioner>
217+
</NavigationMenu.Portal>
218+
</NavigationMenu.Root>
219+
);
220+
}
221+
146222
function TestNavigationMenuWithDialog() {
147223
return (
148224
<NavigationMenu.Root>
@@ -928,6 +1004,90 @@ describe('<NavigationMenu.Root />', () => {
9281004
expect(screen.queryByTestId('nested-popup-2')).not.to.equal(null);
9291005
expect(screen.queryByTestId('nested-popup-1')).to.equal(null);
9301006
});
1007+
1008+
it('allows arrow key navigation to submenu triggers', async () => {
1009+
const { user } = await render(<TestInlineNestedNavigationMenu />);
1010+
const trigger1 = screen.getByTestId('trigger-1');
1011+
1012+
fireEvent.click(trigger1);
1013+
await flushMicrotasks();
1014+
1015+
const popup1 = screen.getByTestId('popup-1');
1016+
expect(popup1).not.to.equal(null);
1017+
1018+
const link1 = screen.getByText('Link 1');
1019+
await act(async () => link1.focus());
1020+
1021+
// Arrow down should move to nested-trigger-1
1022+
await user.keyboard('{ArrowDown}');
1023+
1024+
const nestedTrigger1 = within(popup1).getByTestId('nested-trigger-1');
1025+
expect(nestedTrigger1).toHaveFocus();
1026+
1027+
// Arrow down should move to nested-trigger-2
1028+
await user.keyboard('{ArrowDown}');
1029+
1030+
const nestedTrigger2 = within(popup1).getByTestId('nested-trigger-2');
1031+
expect(nestedTrigger2).toHaveFocus();
1032+
1033+
// Arrow up should move back to nested-trigger-1
1034+
await user.keyboard('{ArrowUp}');
1035+
expect(nestedTrigger1).toHaveFocus();
1036+
1037+
// Arrow up should move back to Link 1
1038+
await user.keyboard('{ArrowUp}');
1039+
expect(link1).toHaveFocus();
1040+
});
1041+
1042+
it('allows arrow key navigation with 3+ levels of nesting', async () => {
1043+
const { user } = await render(<TestDeeplyNestedNavigationMenu />);
1044+
const trigger1 = screen.getByTestId('trigger-1');
1045+
1046+
fireEvent.click(trigger1);
1047+
await flushMicrotasks();
1048+
1049+
const content1 = screen.getByTestId('content-1');
1050+
expect(content1).not.to.equal(null);
1051+
1052+
// Level 1 content contains: Link 1, Level2-trigger-1, Level2-trigger-2
1053+
const link1 = screen.getByTestId('link-1');
1054+
await act(async () => link1.focus());
1055+
1056+
// Navigate through Level 1 content items
1057+
await user.keyboard('{ArrowDown}');
1058+
const level2Trigger1 = screen.getByTestId('level2-trigger-1');
1059+
expect(level2Trigger1).toHaveFocus();
1060+
1061+
await user.keyboard('{ArrowDown}');
1062+
const level2Trigger2 = screen.getByTestId('level2-trigger-2');
1063+
expect(level2Trigger2).toHaveFocus();
1064+
1065+
await user.keyboard('{ArrowUp}');
1066+
expect(level2Trigger1).toHaveFocus();
1067+
1068+
await user.keyboard('{ArrowUp}');
1069+
expect(link1).toHaveFocus();
1070+
1071+
// Now navigate into Level 2 content (which contains Level 3 triggers)
1072+
const level2Content1 = screen.getByTestId('level2-content-1');
1073+
const level2Link1 = within(level2Content1).getByTestId('level2-link-1');
1074+
await act(async () => level2Link1.focus());
1075+
1076+
// Navigate through Level 2 content items (includes Level 3 triggers)
1077+
await user.keyboard('{ArrowDown}');
1078+
const level3Trigger1 = screen.getByTestId('level3-trigger-1');
1079+
expect(level3Trigger1).toHaveFocus();
1080+
1081+
await user.keyboard('{ArrowDown}');
1082+
const level3Trigger2 = screen.getByTestId('level3-trigger-2');
1083+
expect(level3Trigger2).toHaveFocus();
1084+
1085+
await user.keyboard('{ArrowUp}');
1086+
expect(level3Trigger1).toHaveFocus();
1087+
1088+
await user.keyboard('{ArrowUp}');
1089+
expect(level2Link1).toHaveFocus();
1090+
});
9311091
});
9321092
});
9331093
});

packages/react/src/navigation-menu/trigger/NavigationMenuTrigger.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ export const NavigationMenuTrigger = React.forwardRef(function NavigationMenuTri
7979
closeDelay,
8080
orientation,
8181
setViewportInert,
82+
nested,
8283
} = useNavigationMenuRootContext();
8384
const { value: itemValue } = useNavigationMenuItemContext();
8485
const nodeId = useNavigationMenuTreeContext();
@@ -389,6 +390,14 @@ export const NavigationMenuTrigger = React.forwardRef(function NavigationMenuTri
389390
},
390391
onKeyDown(event) {
391392
allowFocusRef.current = true;
393+
394+
// For nested (submenu) triggers, don't intercept arrow keys that are used for
395+
// navigation in the parent content. The arrow keys should be handled by the
396+
// parent's CompositeRoot for navigating between items.
397+
if (nested) {
398+
return;
399+
}
400+
392401
const openHorizontal = orientation === 'horizontal' && event.key === 'ArrowDown';
393402
const openVertical = orientation === 'vertical' && event.key === 'ArrowRight';
394403

0 commit comments

Comments
 (0)