Skip to content

Commit dad2bae

Browse files
authored
Reset internal state when unmounting a submenu (#1503)
1 parent 5d222e5 commit dad2bae

18 files changed

+89
-45
lines changed

dist/es/components/ControlledMenu.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { forwardRef, useRef, useMemo } from 'react';
22
import { createPortal } from 'react-dom';
33
import { MenuList } from './MenuList.js';
44
import { jsx } from 'react/jsx-runtime';
5-
import { Keys, CloseReason, SettingsContext, EventHandlersContext } from '../utils/constants.js';
5+
import { SettingsContext, EventHandlersContext, CloseReason, Keys } from '../utils/constants.js';
66
import { safeCall } from '../utils/utils.js';
77

88
const ControlledMenu = /*#__PURE__*/forwardRef(function ControlledMenu({

dist/es/components/FocusableItem.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { useRef, useContext, useMemo } from 'react';
22
import { jsx } from 'react/jsx-runtime';
33
import { useItemState } from '../hooks/useItemState.js';
4-
import { useCombinedRef } from '../hooks/useCombinedRef.js';
54
import { useBEM } from '../hooks/useBEM.js';
5+
import { useCombinedRef } from '../hooks/useCombinedRef.js';
66
import { withHovering } from '../utils/withHovering.js';
77
import { EventHandlersContext, roleMenuitem, menuClass, menuItemClass } from '../utils/constants.js';
88
import { safeCall, mergeProps, commonProps } from '../utils/utils.js';

dist/es/components/Menu.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { jsxs, jsx } from 'react/jsx-runtime';
44
import { useMenuStateAndFocus } from '../hooks/useMenuStateAndFocus.js';
55
import { useClick } from '../hooks/useClick.js';
66
import { useCombinedRef } from '../hooks/useCombinedRef.js';
7-
import { safeCall, mergeProps, getName, isMenuOpen } from '../utils/utils.js';
7+
import { safeCall, mergeProps, isMenuOpen, getName } from '../utils/utils.js';
88
import { FocusPositions, Keys } from '../utils/constants.js';
99

1010
const isLegacyReact = parseInt(version) < 19;

dist/es/components/MenuContainer.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { useMemo } from 'react';
22
import { jsx } from 'react/jsx-runtime';
3-
import { mergeProps, getTransition, safeCall } from '../utils/utils.js';
43
import { useBEM } from '../hooks/useBEM.js';
5-
import { menuContainerClass, Keys, CloseReason } from '../utils/constants.js';
4+
import { mergeProps, getTransition, safeCall } from '../utils/utils.js';
5+
import { menuContainerClass, CloseReason, Keys } from '../utils/constants.js';
66

77
const MenuContainer = ({
88
className,

dist/es/components/MenuGroup.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ import { forwardRef, useRef, useState, useContext } from 'react';
22
import { jsx } from 'react/jsx-runtime';
33
import { useLayoutEffect as useIsomorphicLayoutEffect } from '../hooks/useIsomorphicLayoutEffect.js';
44
import { getNormalizedClientRect } from '../positionUtils/getNormalizedClientRect.js';
5-
import { useCombinedRef } from '../hooks/useCombinedRef.js';
65
import { useBEM } from '../hooks/useBEM.js';
6+
import { useCombinedRef } from '../hooks/useCombinedRef.js';
77
import { MenuListContext, menuClass, menuGroupClass } from '../utils/constants.js';
88

99
const MenuGroup = /*#__PURE__*/forwardRef(function MenuGroup({

dist/es/components/MenuItem.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import { useContext, useMemo } from 'react';
22
import { jsx } from 'react/jsx-runtime';
33
import { useItemState } from '../hooks/useItemState.js';
4-
import { EventHandlersContext, RadioGroupContext, roleMenuitem, menuClass, menuItemClass, roleNone, Keys } from '../utils/constants.js';
5-
import { useCombinedRef } from '../hooks/useCombinedRef.js';
4+
import { EventHandlersContext, RadioGroupContext, menuClass, menuItemClass, roleNone, roleMenuitem, Keys } from '../utils/constants.js';
65
import { useBEM } from '../hooks/useBEM.js';
6+
import { useCombinedRef } from '../hooks/useCombinedRef.js';
77
import { withHovering } from '../utils/withHovering.js';
8-
import { mergeProps, commonProps, safeCall } from '../utils/utils.js';
8+
import { mergeProps, safeCall, commonProps } from '../utils/utils.js';
99

1010
const MenuItem = /*#__PURE__*/withHovering('MenuItem', function MenuItem({
1111
className,

dist/es/components/MenuList.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ import { flushSync } from 'react-dom';
33
import { MenuContainer } from './MenuContainer.js';
44
import { jsxs, jsx } from 'react/jsx-runtime';
55
import { createSubmenuCtx } from '../utils/submenuCtx.js';
6-
import { SettingsContext, MenuListContext, HoverActionTypes, noScrollFocus, menuClass, menuArrowClass, positionAbsolute, MenuListItemContext, HoverItemContext, Keys, CloseReason, FocusPositions } from '../utils/constants.js';
6+
import { SettingsContext, MenuListContext, HoverActionTypes, noScrollFocus, menuClass, menuArrowClass, positionAbsolute, MenuListItemContext, HoverItemContext, CloseReason, FocusPositions, Keys } from '../utils/constants.js';
77
import { useItems } from '../hooks/useItems.js';
8-
import { getScrollAncestor, commonProps, mergeProps, safeCall, isMenuOpen, getTransition, batchedUpdates } from '../utils/utils.js';
8+
import { getScrollAncestor, isMenuOpen, batchedUpdates, getTransition, mergeProps, commonProps, safeCall } from '../utils/utils.js';
99
import { getPositionHelpers } from '../positionUtils/getPositionHelpers.js';
1010
import { positionMenu } from '../positionUtils/positionMenu.js';
1111
import { useLayoutEffect as useIsomorphicLayoutEffect } from '../hooks/useIsomorphicLayoutEffect.js';

dist/es/components/SubMenu.js

+6-3
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ import { jsxs, jsx } from 'react/jsx-runtime';
55
import { withHovering } from '../utils/withHovering.js';
66
import { useMenuStateAndFocus } from '../hooks/useMenuStateAndFocus.js';
77
import { useItemEffect } from '../hooks/useItemEffect.js';
8+
import { SettingsContext, MenuListContext, MenuListItemContext, roleNone, roleMenuitem, menuClass, menuItemClass, subMenuClass, HoverActionTypes, Keys, FocusPositions } from '../utils/constants.js';
89
import { useBEM } from '../hooks/useBEM.js';
9-
import { SettingsContext, MenuListContext, MenuListItemContext, menuClass, subMenuClass, roleNone, roleMenuitem, menuItemClass, HoverActionTypes, Keys, FocusPositions } from '../utils/constants.js';
1010
import { useCombinedRef } from '../hooks/useCombinedRef.js';
11-
import { mergeProps, commonProps, safeCall, isMenuOpen, batchedUpdates } from '../utils/utils.js';
11+
import { isMenuOpen, mergeProps, commonProps, safeCall, batchedUpdates } from '../utils/utils.js';
1212

1313
const SubMenu = /*#__PURE__*/withHovering('SubMenu', function SubMenu({
1414
'aria-label': ariaLabel,
@@ -115,7 +115,10 @@ const SubMenu = /*#__PURE__*/withHovering('SubMenu', function SubMenu({
115115
};
116116
useItemEffect(isDisabled, itemRef, updateItems);
117117
useEffect(() => submenuCtx.toggle(isOpen), [submenuCtx, isOpen]);
118-
useEffect(() => () => clearTimeout(timerId.v), [timerId]);
118+
useEffect(() => () => {
119+
clearTimeout(timerId.v);
120+
submenuCtx.toggle(false);
121+
}, [timerId, submenuCtx]);
119122
useEffect(() => {
120123
if (isHovering && isParentOpen) {
121124
itemRef.current.focus();

dist/es/hooks/useMenuState.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { useCallback } from 'react';
22
import { useTransitionState } from 'react-transition-state';
3-
import { MenuStateMap } from '../utils/constants.js';
43
import { getTransition } from '../utils/utils.js';
4+
import { MenuStateMap } from '../utils/constants.js';
55

66
const useMenuState = ({
77
initialOpen,

dist/es/utils/withHovering.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { forwardRef, useRef, useContext, memo } from 'react';
1+
import { forwardRef, useRef, memo, useContext } from 'react';
22
import { HoverItemContext } from './constants.js';
33
import { jsx } from 'react/jsx-runtime';
44

dist/index.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -1469,7 +1469,10 @@ const SubMenu = /*#__PURE__*/withHovering('SubMenu', function SubMenu({
14691469
};
14701470
useItemEffect(isDisabled, itemRef, updateItems);
14711471
react.useEffect(() => submenuCtx.toggle(isOpen), [submenuCtx, isOpen]);
1472-
react.useEffect(() => () => clearTimeout(timerId.v), [timerId]);
1472+
react.useEffect(() => () => {
1473+
clearTimeout(timerId.v);
1474+
submenuCtx.toggle(false);
1475+
}, [timerId, submenuCtx]);
14731476
react.useEffect(() => {
14741477
if (isHovering && isParentOpen) {
14751478
itemRef.current.focus();

example/package-lock.json

+11-11
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

example/src/utils/index.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { useEffect, useLayoutEffect } from 'react';
22
import { useTheme } from '../store';
33

4-
export const version = '4.3.0';
5-
export const build = '139';
4+
export const version = '4.3.1';
5+
export const build = '140';
66

77
export const bem = (block, element, modifiers = {}) => {
88
let blockElement = element ? `${block}__${element}` : block;

package-lock.json

+12-12
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@szhsin/react-menu",
3-
"version": "4.3.0",
3+
"version": "4.3.1",
44
"description": "React component for building accessible menu, dropdown, submenu, context menu and more.",
55
"author": "Zheng Song",
66
"license": "MIT",

src/__tests__/SubMenu.test.js

+27
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,33 @@ test('Delay closing submenu when hovering items in parent menu list', async () =
226226
utils.expectMenuToBeOpen(false, submenuOptions1);
227227
});
228228

229+
test('Unmounting an open submenu should reset the internal state and stop introducing delays when hovering over sibling items.', () => {
230+
const { rerender } = render(
231+
<Menu menuButton={<MenuButton>Menu</MenuButton>}>
232+
<MenuItem>1</MenuItem>
233+
<SubMenu label="Submenu">
234+
<MenuItem>1.1</MenuItem>
235+
<MenuItem>1.2</MenuItem>
236+
</SubMenu>
237+
</Menu>
238+
);
239+
240+
utils.clickMenuButton();
241+
fireEvent.click(utils.queryMenuItem('Submenu'));
242+
utils.expectMenuToBeOpen(true, { name: 'Submenu' });
243+
244+
rerender(
245+
<Menu menuButton={<MenuButton>Menu</MenuButton>}>
246+
<MenuItem>1</MenuItem>
247+
<MenuItem>2</MenuItem>
248+
</Menu>
249+
);
250+
251+
const item = utils.queryMenuItem('1');
252+
fireEvent.pointerMove(item);
253+
utils.expectMenuItemToBeHover(item, true);
254+
});
255+
229256
test('openTrigger is "clickOnly"', async () => {
230257
renderMenu(null, null, { openTrigger: 'clickOnly' });
231258
utils.clickMenuButton();

src/components/MenuList.js

+2
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,8 @@ export const MenuList = ({
410410
{...commonProps(isDisabled)}
411411
{...mergeProps(
412412
{
413+
// Moves the cursor from a non-item area in the parent menu to the current menu,
414+
// without fully leaving any menu, so the onPointerLeave event is not triggered.
413415
onPointerEnter: parentSubmenuCtx?.off,
414416
onPointerMove,
415417
onPointerLeave,

src/components/SubMenu.js

+11-2
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,13 @@ export const SubMenu = withHovering(
129129

130130
useItemEffect(isDisabled, itemRef, updateItems);
131131
useEffect(() => submenuCtx.toggle(isOpen), [submenuCtx, isOpen]);
132-
useEffect(() => () => clearTimeout(timerId.v), [timerId]);
132+
useEffect(
133+
() => () => {
134+
clearTimeout(timerId.v);
135+
submenuCtx.toggle(false);
136+
},
137+
[timerId, submenuCtx]
138+
);
133139
useEffect(() => {
134140
// Don't set focus when parent menu is closed, otherwise focus will be lost
135141
// and onBlur event will be fired with relatedTarget setting as null.
@@ -166,7 +172,10 @@ export const SubMenu = withHovering(
166172

167173
const mergedItemProps = mergeProps(
168174
{
169-
onPointerEnter: submenuCtx.off, // For moving mouse from submenu back to its anchor item
175+
// For moving the cursor from a submenu back to its anchor item,
176+
// crossing over a non-item area in the parent menu.
177+
onPointerEnter: submenuCtx.off,
178+
170179
onPointerMove,
171180
onPointerLeave,
172181
onKeyDown,

0 commit comments

Comments
 (0)