Skip to content

Commit 6a89a8a

Browse files
atomiksatomiks
committed
[menu] Fix submenu stuck glitch
1 parent c1c7f67 commit 6a89a8a

File tree

7 files changed

+249
-78
lines changed

7 files changed

+249
-78
lines changed

‎docs/src/app/(private)/experiments/menu/menu-submenus.tsx‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
'use client';
2+
import * as React from 'react';
3+
import { Menu } from '@base-ui/react/menu';
4+
import {
5+
SettingsMetadata,
6+
useExperimentSettings,
7+
} from '../../../../components/Experiments/SettingsPanel';
8+
import '../../../../demo-theme.css';
9+
import classes from './menu.module.css';
10+
11+
interface Settings {
12+
customAnchor: boolean;
13+
modal: boolean;
14+
openOnHover: boolean;
15+
disabled: boolean;
16+
customTriggerElement: boolean;
17+
side: Menu.Positioner.Props['side'];
18+
align: Menu.Positioner.Props['align'];
19+
}
20+
21+
export default function MenuSubmenus() {
22+
const { settings } = useExperimentSettings<Settings>();
23+
24+
const anchorRef = React.useRef<HTMLDivElement>(null);
25+
26+
const triggerRender = React.useMemo(
27+
() => (settings.customTriggerElement ? <span /> : undefined),
28+
[settings.customTriggerElement],
29+
);
30+
31+
const handleItemClick = React.useCallback((event: React.MouseEvent<HTMLDivElement>) => {
32+
console.log(`${event.currentTarget.textContent} clicked`);
33+
}, []);
34+
35+
return (
36+
<div>
37+
<h1>Many adjacent submenus</h1>
38+
<Menu.Root modal={settings.modal} disabled={settings.disabled}>
39+
<Menu.Trigger
40+
className={classes.Button}
41+
render={triggerRender}
42+
nativeButton={triggerRender === undefined}
43+
openOnHover={settings.openOnHover}
44+
>
45+
Menu <ChevronDownIcon className={classes.ButtonIcon} />
46+
</Menu.Trigger>
47+
<Menu.Portal>
48+
<Menu.Positioner
49+
className={classes.Positioner}
50+
sideOffset={8}
51+
anchor={settings.customAnchor ? anchorRef : undefined}
52+
side={settings.side}
53+
align={settings.align}
54+
>
55+
<Menu.Popup
56+
className={classes.Popup}
57+
style={{ maxHeight: 'var(--available-height)', overflowY: 'scroll' }}
58+
>
59+
{Array.from({ length: 50 }).map((_, submenuIndex) => (
60+
<Menu.SubmenuRoot key={submenuIndex}>
61+
<Menu.SubmenuTrigger className={classes.SubmenuTrigger}>
62+
Submenu test index {submenuIndex + 1}
63+
<ChevronRightIcon />
64+
</Menu.SubmenuTrigger>
65+
<Menu.Portal>
66+
<Menu.Positioner className={classes.Positioner} sideOffset={8}>
67+
<Menu.Popup className={classes.Popup}>
68+
{Array.from({ length: 20 }).map((__, itemIndex) => (
69+
<Menu.Item
70+
key={itemIndex}
71+
className={classes.Item}
72+
onClick={handleItemClick}
73+
>
74+
Submenu test index {submenuIndex + 1} - Item {itemIndex + 1}
75+
</Menu.Item>
76+
))}
77+
</Menu.Popup>
78+
</Menu.Positioner>
79+
</Menu.Portal>
80+
</Menu.SubmenuRoot>
81+
))}
82+
</Menu.Popup>
83+
</Menu.Positioner>
84+
</Menu.Portal>
85+
</Menu.Root>
86+
87+
{settings.customAnchor && (
88+
<div className={classes.CustomAnchor} ref={anchorRef}>
89+
Menu will be anchored here
90+
</div>
91+
)}
92+
</div>
93+
);
94+
}
95+
96+
export const settingsMetadata: SettingsMetadata<Settings> = {
97+
customAnchor: {
98+
type: 'boolean',
99+
label: 'Custom anchor',
100+
},
101+
modal: {
102+
type: 'boolean',
103+
label: 'Modal',
104+
default: true,
105+
},
106+
openOnHover: {
107+
type: 'boolean',
108+
label: 'Open on hover',
109+
},
110+
disabled: {
111+
type: 'boolean',
112+
label: 'Disabled',
113+
},
114+
customTriggerElement: {
115+
type: 'boolean',
116+
label: 'Trigger as <span>',
117+
},
118+
side: {
119+
type: 'string',
120+
label: 'Side',
121+
options: ['top', 'right', 'bottom', 'left'],
122+
default: 'bottom',
123+
},
124+
align: {
125+
type: 'string',
126+
label: 'Align',
127+
options: ['start', 'center', 'end'],
128+
default: 'center',
129+
},
130+
};
131+
132+
function ChevronDownIcon(props: React.ComponentProps<'svg'>) {
133+
return (
134+
<svg width="10" height="10" viewBox="0 0 10 10" fill="none" {...props}>
135+
<path d="M1 3.5L5 7.5L9 3.5" stroke="currentcolor" strokeWidth="1.5" />
136+
</svg>
137+
);
138+
}
139+
140+
function ChevronRightIcon(props: React.ComponentProps<'svg'>) {
141+
return (
142+
<svg width="10" height="10" viewBox="0 0 10 10" fill="none" {...props}>
143+
<path d="M3.5 9L7.5 5L3.5 1" stroke="currentcolor" strokeWidth="1.5" />
144+
</svg>
145+
);
146+
}

‎packages/react/src/menu/backdrop/MenuBackdrop.test.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
import { Menu } from '@base-ui/react/menu';
22
import { createRenderer, describeConformance } from '#test-utils';
3-
import { screen, waitFor } from '@mui/internal-test-utils';
3+
import { fireEvent, screen, waitFor } from '@mui/internal-test-utils';
44

55
describe('<Menu.Backdrop />', () => {
66
const { render } = createRenderer();
77

8+
async function hoverWithMouseMove(element: Element, user: { hover: (node: Element) => Promise<void> }) {
9+
fireEvent.mouseMove(element);
10+
await user.hover(element);
11+
}
12+
813
describeConformance(<Menu.Backdrop />, () => ({
914
refInstanceof: window.HTMLDivElement,
1015
render(node) {
@@ -27,7 +32,7 @@ describe('<Menu.Backdrop />', () => {
2732
</Menu.Root>,
2833
);
2934

30-
await user.hover(screen.getByText('Open'));
35+
await hoverWithMouseMove(screen.getByText('Open'), user);
3136

3237
expect(screen.getByTestId('backdrop').style.pointerEvents).to.equal('none');
3338
});

‎packages/react/src/menu/root/MenuRoot.detached-triggers.test.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ import { spy } from 'sinon';
55
import { Menu } from '@base-ui/react/menu';
66
import { createRenderer, isJSDOM, wait } from '#test-utils';
77

8+
type HoverUser = {
9+
hover: (element: Element) => Promise<void>;
10+
};
11+
12+
async function hoverWithMouseMove(element: Element, user: HoverUser) {
13+
fireEvent.mouseMove(element);
14+
await user.hover(element);
15+
}
16+
817
describe('<MenuRoot />', () => {
918
beforeEach(() => {
1019
globalThis.BASE_UI_ANIMATIONS_DISABLED = true;
@@ -457,7 +466,7 @@ describe('<MenuRoot />', () => {
457466
await screen.findByTestId('menu');
458467

459468
const submenuTrigger = await screen.findByTestId('submenu-trigger');
460-
await user.hover(submenuTrigger);
469+
await hoverWithMouseMove(submenuTrigger, user);
461470
await screen.findByTestId('submenu');
462471

463472
// Wait 200ms to enable mouseup on menu items
@@ -954,7 +963,7 @@ describe('<MenuRoot />', () => {
954963
await screen.findByTestId('menu');
955964

956965
const submenuTrigger = await screen.findByTestId('submenu-trigger');
957-
await user.hover(submenuTrigger);
966+
await hoverWithMouseMove(submenuTrigger, user);
958967
await screen.findByTestId('submenu');
959968

960969
// Wait 200ms to enable mouseup on menu items

‎packages/react/src/menu/root/MenuRoot.test.tsx‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@ import { createRenderer, isJSDOM, popupConformanceTests, wait } from '#test-util
1818
import { REASONS } from '../../utils/reasons';
1919
import { PATIENT_CLICK_THRESHOLD } from '../../utils/constants';
2020

21+
type HoverUser = {
22+
hover: (element: Element) => Promise<void>;
23+
};
24+
25+
async function hoverWithMouseMove(element: Element, user: HoverUser = userEvent) {
26+
fireEvent.mouseMove(element);
27+
await user.hover(element);
28+
}
29+
2130
describe('<Menu.Root />', () => {
2231
beforeEach(() => {
2332
globalThis.BASE_UI_ANIMATIONS_DISABLED = true;
@@ -472,7 +481,7 @@ describe('<Menu.Root />', () => {
472481
expect(screen.queryByTestId('submenu')).to.equal(null);
473482

474483
const submenuTrigger = await screen.findByTestId('submenu-trigger');
475-
await user.hover(submenuTrigger);
484+
await hoverWithMouseMove(submenuTrigger, user);
476485

477486
await waitFor(() => {
478487
expect(screen.queryByTestId('submenu')).not.to.equal(null);
@@ -1108,7 +1117,7 @@ describe('<Menu.Root />', () => {
11081117
trigger.focus();
11091118
});
11101119

1111-
await userEvent.hover(trigger);
1120+
await hoverWithMouseMove(trigger);
11121121

11131122
await waitFor(() => {
11141123
expect(screen.queryByRole('menu')).not.to.equal(null);
@@ -1126,7 +1135,7 @@ describe('<Menu.Root />', () => {
11261135
trigger.focus();
11271136
});
11281137

1129-
await userEvent.hover(trigger);
1138+
await hoverWithMouseMove(trigger);
11301139

11311140
await waitFor(() => {
11321141
expect(screen.queryByRole('menu')).not.to.equal(null);
@@ -1149,10 +1158,11 @@ describe('<Menu.Root />', () => {
11491158

11501159
const submenuTrigger = screen.getByTestId('submenu-trigger');
11511160

1152-
fireEvent.mouseEnter(submenuTrigger);
1153-
fireEvent.mouseMove(submenuTrigger);
1161+
await hoverWithMouseMove(submenuTrigger);
11541162

1155-
expect(screen.queryByTestId('submenu')).not.to.equal(null);
1163+
await waitFor(() => {
1164+
expect(screen.queryByTestId('submenu')).not.to.equal(null);
1165+
});
11561166
});
11571167

11581168
it('should not close when submenu is hovered after root menu is hovered', async () => {
@@ -1169,19 +1179,19 @@ describe('<Menu.Root />', () => {
11691179
trigger.focus();
11701180
});
11711181

1172-
await userEvent.hover(trigger);
1182+
await hoverWithMouseMove(trigger);
11731183

11741184
await waitFor(() => {
11751185
expect(screen.getByTestId('menu')).not.to.equal(null);
11761186
});
11771187

11781188
const menu = screen.getByTestId('menu');
11791189

1180-
await userEvent.hover(menu);
1190+
await hoverWithMouseMove(menu);
11811191

11821192
const submenuTrigger = screen.getByRole('menuitem', { name: 'Item 4' });
11831193

1184-
await userEvent.hover(submenuTrigger);
1194+
await hoverWithMouseMove(submenuTrigger);
11851195

11861196
await waitFor(() => {
11871197
expect(screen.getByTestId('menu')).not.to.equal(null);
@@ -1195,7 +1205,7 @@ describe('<Menu.Root />', () => {
11951205
// Use fireEvent to bypass pointer-events checks during safe-polygon pointer events mutation
11961206
fireEvent.mouseMove(menu);
11971207
fireEvent.mouseLeave(menu);
1198-
await userEvent.hover(submenu);
1208+
await hoverWithMouseMove(submenu);
11991209

12001210
await waitFor(() => {
12011211
expect(screen.getByTestId('menu')).not.to.equal(null);
@@ -1219,23 +1229,23 @@ describe('<Menu.Root />', () => {
12191229
trigger.focus();
12201230
});
12211231

1222-
await userEvent.hover(trigger);
1232+
await hoverWithMouseMove(trigger);
12231233

12241234
await waitFor(() => {
12251235
expect(screen.getByTestId('menu')).not.to.equal(null);
12261236
});
12271237

12281238
// Open first-level submenu
12291239
const level1Trigger = screen.getByRole('menuitem', { name: 'Item 4' });
1230-
await userEvent.hover(level1Trigger);
1240+
await hoverWithMouseMove(level1Trigger);
12311241

12321242
await waitFor(() => {
12331243
expect(screen.getByTestId('submenu')).not.to.equal(null);
12341244
});
12351245

12361246
// Open second-level submenu
12371247
const level2Trigger = screen.getByRole('menuitem', { name: 'Item 4.3' });
1238-
await userEvent.hover(level2Trigger);
1248+
await hoverWithMouseMove(level2Trigger);
12391249

12401250
await waitFor(() => {
12411251
expect(screen.getByTestId('nested-submenu')).not.to.equal(null);

‎packages/react/src/menu/store/MenuStore.ts‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,7 @@ const selectors = {
5757
(state.modal ?? true),
5858
),
5959

60-
allowMouseEnter: createSelector((state: State<unknown>): boolean =>
61-
state.parent.type === 'menu'
62-
? state.parent.store.select('allowMouseEnter')
63-
: state.allowMouseEnter,
64-
),
60+
allowMouseEnter: createSelector((state: State<unknown>) => state.allowMouseEnter),
6561
stickIfOpen: createSelector((state: State<unknown>) => state.stickIfOpen),
6662
parent: createSelector((state: State<unknown>) => state.parent),
6763
rootId: createSelector((state: State<unknown>): string | undefined => {
@@ -127,18 +123,6 @@ export class MenuStore<Payload> extends ReactStore<
127123
selectors,
128124
);
129125

130-
// Sync `allowMouseEnter` with parent menu if applicable.
131-
this.observe(
132-
createSelector((state) => state.allowMouseEnter),
133-
(allowMouseEnter, oldValue) => {
134-
// The allowMouseEnter !== oldValue check prevent calling parent store's set
135-
// on intialization. Without it, React might complain about updating one component during rendering another.
136-
if (this.state.parent.type === 'menu' && allowMouseEnter !== oldValue) {
137-
this.state.parent.store.set('allowMouseEnter', allowMouseEnter);
138-
}
139-
},
140-
);
141-
142126
// Set up propagation of state from parent menu if applicable.
143127
this.unsubscribeParentListener = this.observe('parent', (parent) => {
144128
this.unsubscribeParentListener?.();
@@ -184,7 +168,7 @@ function createInitialState<Payload>(): State<Payload> {
184168
...createInitialPopupStoreState(),
185169
disabled: false,
186170
modal: true,
187-
allowMouseEnter: true,
171+
allowMouseEnter: false,
188172
stickIfOpen: true,
189173
parent: {
190174
type: undefined,

‎packages/react/src/menu/submenu-trigger/MenuSubmenuTrigger.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export const MenuSubmenuTrigger = React.forwardRef(function SubmenuTriggerCompon
112112
});
113113

114114
const hoverEnabled = store.useState('hoverEnabled');
115-
const allowMouseEnter = store.useState('allowMouseEnter');
115+
const allowMouseEnter = parentMenuStore.useState('allowMouseEnter');
116116

117117
const hoverProps = useHoverReferenceInteraction(floatingRootContext, {
118118
enabled: hoverEnabled && openOnHover && !disabled && allowMouseEnter,

0 commit comments

Comments
 (0)