Skip to content

Commit 6c8af38

Browse files
committed
[context menu] Ensure submenus close when root closes
1 parent 3dbc0e3 commit 6c8af38

File tree

6 files changed

+213
-14
lines changed

6 files changed

+213
-14
lines changed
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
import * as React from 'react';
2+
import { expect } from 'chai';
3+
import { fireEvent, screen, waitFor } from '@mui/internal-test-utils';
4+
import { spy } from 'sinon';
5+
import { ContextMenu } from '@base-ui-components/react/context-menu';
6+
import { createRenderer } from '#test-utils';
7+
8+
describe('<ContextMenu.Root />', () => {
9+
beforeEach(() => {
10+
globalThis.BASE_UI_ANIMATIONS_DISABLED = true;
11+
});
12+
13+
const { render, clock } = createRenderer({
14+
clockOptions: {
15+
shouldAdvanceTime: true,
16+
},
17+
});
18+
19+
describe('interactions', () => {
20+
clock.withFakeTimers();
21+
22+
it('closes nested submenus when releasing the context menu pointer over an item', async () => {
23+
const rootOnOpenChange = spy();
24+
const submenuOnOpenChange = spy();
25+
26+
const { user } = await render(
27+
<ContextMenu.Root onOpenChange={rootOnOpenChange}>
28+
<ContextMenu.Trigger data-testid="context-trigger">Surface</ContextMenu.Trigger>
29+
<ContextMenu.Portal>
30+
<ContextMenu.Positioner>
31+
<ContextMenu.Popup data-testid="context-root-popup">
32+
<ContextMenu.SubmenuRoot defaultOpen onOpenChange={submenuOnOpenChange} delay={1}>
33+
<ContextMenu.SubmenuTrigger data-testid="context-submenu-trigger">
34+
More options
35+
</ContextMenu.SubmenuTrigger>
36+
<ContextMenu.Portal>
37+
<ContextMenu.Positioner>
38+
<ContextMenu.Popup data-testid="context-submenu-popup">
39+
<ContextMenu.Item data-testid="context-submenu-item">
40+
Deep action
41+
</ContextMenu.Item>
42+
</ContextMenu.Popup>
43+
</ContextMenu.Positioner>
44+
</ContextMenu.Portal>
45+
</ContextMenu.SubmenuRoot>
46+
</ContextMenu.Popup>
47+
</ContextMenu.Positioner>
48+
</ContextMenu.Portal>
49+
</ContextMenu.Root>,
50+
);
51+
52+
const trigger = screen.getByTestId('context-trigger');
53+
54+
fireEvent.contextMenu(trigger, { clientX: 10, clientY: 10, button: 2 });
55+
56+
await screen.findByTestId('context-root-popup');
57+
58+
const submenuTrigger = screen.getByTestId('context-submenu-trigger');
59+
await user.hover(submenuTrigger);
60+
61+
await screen.findByTestId('context-submenu-popup');
62+
63+
const submenuItem = screen.getByTestId('context-submenu-item');
64+
fireEvent.mouseUp(submenuItem, { button: 2 });
65+
66+
await waitFor(() => {
67+
expect(screen.queryByTestId('context-submenu-popup')).to.equal(null);
68+
});
69+
70+
await waitFor(() => {
71+
expect(screen.queryByTestId('context-root-popup')).to.equal(null);
72+
});
73+
74+
expect(submenuOnOpenChange.lastCall?.args[0]).to.equal(false);
75+
expect(submenuOnOpenChange.lastCall?.args[1].reason).to.equal('item-press');
76+
expect(rootOnOpenChange.lastCall?.args[0]).to.equal(false);
77+
expect(rootOnOpenChange.lastCall?.args[1].reason).to.equal('item-press');
78+
});
79+
});
80+
});

packages/react/src/context-menu/trigger/ContextMenuTrigger.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { BaseUIComponentProps } from '../../utils/types';
88
import { useContextMenuRootContext } from '../root/ContextMenuRootContext';
99
import { useRenderElement } from '../../utils/useRenderElement';
1010
import { createBaseUIEventDetails } from '../../utils/createBaseUIEventDetails';
11+
import { findRootOwnerId } from '../../menu/utils/findRootOwnerId';
1112

1213
const LONG_PRESS_DELAY = 500;
1314

@@ -30,6 +31,7 @@ export const ContextMenuTrigger = React.forwardRef(function ContextMenuTrigger(
3031
backdropRef,
3132
positionerRef,
3233
allowMouseUpTriggerRef,
34+
rootId,
3335
} = useContextMenuRootContext(false);
3436

3537
const triggerRef = React.useRef<HTMLDivElement | null>(null);
@@ -80,7 +82,13 @@ export const ContextMenuTrigger = React.forwardRef(function ContextMenuTrigger(
8082
allowMouseUpTimeout.clear();
8183
allowMouseUpRef.current = false;
8284

83-
if (contains(positionerRef.current, getTarget(mouseEvent) as Element | null)) {
85+
const mouseUpTarget = getTarget(mouseEvent) as Element | null;
86+
87+
if (contains(positionerRef.current, mouseUpTarget)) {
88+
return;
89+
}
90+
91+
if (rootId && mouseUpTarget && findRootOwnerId(mouseUpTarget) === rootId) {
8492
return;
8593
}
8694

packages/react/src/menu/positioner/MenuPositioner.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from '../../floating-ui-react';
1010
import { MenuPositionerContext } from './MenuPositionerContext';
1111
import { useMenuRootContext } from '../root/MenuRootContext';
12+
import type { MenuRoot } from '../root/MenuRoot';
1213
import { useAnchorPositioning, type Align, type Side } from '../../utils/useAnchorPositioning';
1314
import { useRenderElement } from '../../utils/useRenderElement';
1415
import { BaseUIComponentProps } from '../../utils/types';
@@ -160,6 +161,27 @@ export const MenuPositioner = React.forwardRef(function MenuPositioner(
160161
};
161162
}, [menuEvents, nodeId, parentNodeId, setOpen, setHoverEnabled]);
162163

164+
React.useEffect(() => {
165+
if (parentNodeId == null) {
166+
return undefined;
167+
}
168+
169+
function onParentClose(details: MenuOpenEventDetails) {
170+
if (details.open || details.nodeId !== parentNodeId) {
171+
return;
172+
}
173+
174+
const reason: MenuRoot.ChangeEventReason = details.reason ?? 'sibling-open';
175+
setOpen(false, createBaseUIEventDetails(reason));
176+
}
177+
178+
menuEvents.on('menuopenchange', onParentClose);
179+
180+
return () => {
181+
menuEvents.off('menuopenchange', onParentClose);
182+
};
183+
}, [menuEvents, parentNodeId, setOpen]);
184+
163185
// Close unrelated child submenus when hovering a different item in the parent menu.
164186
React.useEffect(() => {
165187
function onItemHover(event: { nodeId: string | undefined; target: Element | null }) {

packages/react/src/menu/trigger/MenuTrigger.tsx

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
'use client';
22
import * as React from 'react';
3-
import { getParentNode, isHTMLElement, isLastTraversableNode } from '@floating-ui/utils/dom';
43
import { useMergedRefs } from '@base-ui-components/utils/useMergedRefs';
54
import { useTimeout } from '@base-ui-components/utils/useTimeout';
65
import { ownerDocument } from '@base-ui-components/utils/owner';
@@ -15,6 +14,7 @@ import { mergeProps } from '../../merge-props';
1514
import { useButton } from '../../use-button/useButton';
1615
import { getPseudoElementBounds } from '../../utils/getPseudoElementBounds';
1716
import { CompositeItem } from '../../composite/item/CompositeItem';
17+
import { findRootOwnerId } from '../utils/findRootOwnerId';
1818

1919
const BOUNDARY_OFFSET = 2;
2020

@@ -201,15 +201,3 @@ export namespace MenuTrigger {
201201
open: boolean;
202202
};
203203
}
204-
205-
function findRootOwnerId(node: Node): string | undefined {
206-
if (isHTMLElement(node) && node.hasAttribute('data-rootownerid')) {
207-
return node.getAttribute('data-rootownerid') ?? undefined;
208-
}
209-
210-
if (isLastTraversableNode(node)) {
211-
return undefined;
212-
}
213-
214-
return findRootOwnerId(getParentNode(node));
215-
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { getParentNode, isHTMLElement, isLastTraversableNode } from '@floating-ui/utils/dom';
2+
3+
export function findRootOwnerId(node: Node): string | undefined {
4+
if (isHTMLElement(node) && node.hasAttribute('data-rootownerid')) {
5+
return node.getAttribute('data-rootownerid') ?? undefined;
6+
}
7+
8+
if (isLastTraversableNode(node)) {
9+
return undefined;
10+
}
11+
12+
return findRootOwnerId(getParentNode(node));
13+
}

packages/react/src/menubar/Menubar.test.tsx

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as React from 'react';
22
import { expect } from 'chai';
33
import { act, screen, waitFor } from '@mui/internal-test-utils';
44
import { createRenderer, describeConformance, isJSDOM } from '#test-utils';
5+
import { spy } from 'sinon';
56
import { afterEach } from 'vitest';
67
import { Menubar } from '@base-ui-components/react/menubar';
78
import { Menu } from '@base-ui-components/react/menu';
@@ -469,6 +470,93 @@ describe('<Menubar />', () => {
469470
expect(shareTrigger).toHaveFocus();
470471
});
471472

473+
it('closes open submenus when navigating to the next menubar item with ArrowRight', async () => {
474+
const rootOnOpenChange = spy();
475+
const submenuOnOpenChange = spy();
476+
const nextOnOpenChange = spy();
477+
478+
function SpyMenubar() {
479+
return (
480+
<Menubar>
481+
<Menu.Root onOpenChange={rootOnOpenChange}>
482+
<Menu.Trigger data-testid="menubar-file-trigger">File</Menu.Trigger>
483+
<Menu.Portal>
484+
<Menu.Positioner data-testid="menubar-file-menu">
485+
<Menu.Popup>
486+
<Menu.Item data-testid="menubar-file-item">Item 1</Menu.Item>
487+
<Menu.SubmenuRoot onOpenChange={submenuOnOpenChange}>
488+
<Menu.SubmenuTrigger data-testid="menubar-submenu-trigger">
489+
Share
490+
</Menu.SubmenuTrigger>
491+
<Menu.Portal>
492+
<Menu.Positioner data-testid="menubar-submenu-menu">
493+
<Menu.Popup>
494+
<Menu.Item data-testid="menubar-submenu-item">Email</Menu.Item>
495+
</Menu.Popup>
496+
</Menu.Positioner>
497+
</Menu.Portal>
498+
</Menu.SubmenuRoot>
499+
</Menu.Popup>
500+
</Menu.Positioner>
501+
</Menu.Portal>
502+
</Menu.Root>
503+
<Menu.Root onOpenChange={nextOnOpenChange}>
504+
<Menu.Trigger data-testid="menubar-next-trigger">Edit</Menu.Trigger>
505+
<Menu.Portal>
506+
<Menu.Positioner data-testid="menubar-next-menu">
507+
<Menu.Popup>
508+
<Menu.Item>Edit Item</Menu.Item>
509+
</Menu.Popup>
510+
</Menu.Positioner>
511+
</Menu.Portal>
512+
</Menu.Root>
513+
</Menubar>
514+
);
515+
}
516+
517+
const { user } = await render(<SpyMenubar />);
518+
const fileTrigger = screen.getByTestId('menubar-file-trigger');
519+
520+
await act(async () => {
521+
fileTrigger.focus();
522+
});
523+
await user.keyboard('{Enter}');
524+
await screen.findByTestId('menubar-file-menu');
525+
526+
await waitFor(() => {
527+
expect(screen.getByTestId('menubar-file-item')).toHaveFocus();
528+
});
529+
530+
await user.keyboard('{ArrowDown}');
531+
const submenuTrigger = screen.getByTestId('menubar-submenu-trigger');
532+
await waitFor(() => {
533+
expect(submenuTrigger).toHaveFocus();
534+
});
535+
536+
await user.keyboard('{ArrowRight}');
537+
await screen.findByTestId('menubar-submenu-menu');
538+
539+
await waitFor(() => {
540+
expect(screen.getByTestId('menubar-submenu-item')).toHaveFocus();
541+
});
542+
543+
await user.keyboard('{ArrowRight}');
544+
545+
await screen.findByTestId('menubar-next-menu');
546+
547+
await waitFor(() => {
548+
expect(screen.queryByTestId('menubar-submenu-menu')).to.equal(null);
549+
});
550+
551+
await waitFor(() => {
552+
expect(screen.queryByTestId('menubar-file-menu')).to.equal(null);
553+
});
554+
555+
expect(submenuOnOpenChange.lastCall?.args[0]).to.equal(false);
556+
expect(rootOnOpenChange.lastCall?.args[0]).to.equal(false);
557+
expect(nextOnOpenChange.lastCall?.args[0]).to.equal(true);
558+
});
559+
472560
// Doesn't work in headless mode.
473561
it.skipIf(!isJSDOM)(
474562
'should navigate between menus using left/right arrow keys when menus are open',

0 commit comments

Comments
 (0)