Skip to content

Commit 07260d1

Browse files
authored
Make all submenus close upon user interact outside (#4988)
* Make all submenus close upon user interact outside * add test * refactor to close on underlay press * turns out we dont need the ref * restrict to spectrum for now adding the logic to useOverlay brings up the possibility where a user could have a dismissible overlay open a non dismissible overlay but the absense of a underlay for the non dismissible overlay would then make the click on the dismissible overlay's underlay close both overlays which is different behavior from before. Isolating this change to spectrum for our submenu case until we implement something like focusscope tree to detect clicks outside the tree of menus
1 parent 2600c96 commit 07260d1

File tree

3 files changed

+34
-3
lines changed

3 files changed

+34
-3
lines changed

packages/@react-spectrum/menu/test/MenuTrigger.test.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1177,6 +1177,33 @@ describe('MenuTrigger', function () {
11771177
let dialog = tree.getByRole('dialog');
11781178
expect(dialog).toBeVisible();
11791179
});
1180+
1181+
it('should close everything if the user clicks on the underlay of the root menu', function () {
1182+
renderTree();
1183+
let menu = openMenu();
1184+
let menuItems = within(menu).getAllByRole('menuitem');
1185+
let unavailableItem = menuItems[4];
1186+
expect(unavailableItem).toBeVisible();
1187+
expect(unavailableItem).toHaveAttribute('aria-haspopup', 'dialog');
1188+
1189+
fireEvent.mouseEnter(unavailableItem);
1190+
act(() => {jest.runAllTimers();});
1191+
let dialog = tree.getByRole('dialog');
1192+
expect(dialog).toBeVisible();
1193+
1194+
expect(document.activeElement).toBe(dialog);
1195+
1196+
let underlay = tree.getByTestId('underlay', {hidden: true});
1197+
fireEvent.pointerDown(underlay);
1198+
fireEvent.pointerUp(underlay);
1199+
act(() => {jest.runAllTimers();});
1200+
act(() => {jest.runAllTimers();});
1201+
expect(dialog).not.toBeInTheDocument();
1202+
expect(menu).not.toBeInTheDocument();
1203+
1204+
let triggerButton = tree.getByRole('button');
1205+
expect(document.activeElement).toBe(triggerButton);
1206+
});
11801207
});
11811208
});
11821209
});

packages/@react-spectrum/overlays/src/Popover.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,14 @@ const PopoverWrapper = forwardRef((props: PopoverWrapperProps, ref: RefObject<HT
115115
}, state);
116116
let {focusWithinProps} = useFocusWithin(props);
117117

118+
let onPointerDown = () => {
119+
state.close();
120+
};
121+
118122
// Attach Transition's nodeRef to outermost wrapper for node.reflow: https://github.com/reactjs/react-transition-group/blob/c89f807067b32eea6f68fd6c622190d88ced82e2/src/Transition.js#L231
119123
return (
120124
<div ref={wrapperRef}>
121-
{!isNonModal && <Underlay isTransparent {...underlayProps} isOpen={isOpen} /> }
125+
{!isNonModal && <Underlay isTransparent {...mergeProps(underlayProps, {onPointerDown})} isOpen={isOpen} /> }
122126
<div
123127
{...styleProps}
124128
{...mergeProps(popoverProps, focusWithinProps)}

packages/@react-spectrum/overlays/src/Underlay.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ interface UnderlayProps {
1919
isTransparent?: boolean
2020
}
2121

22-
export function Underlay({isOpen, isTransparent}: UnderlayProps) {
22+
export function Underlay({isOpen, isTransparent, ...otherProps}: UnderlayProps) {
2323
return (
24-
<div className={classNames(underlayStyles, 'spectrum-Underlay', {'is-open': isOpen, 'spectrum-Underlay--transparent': isTransparent})} />
24+
<div data-testid="underlay" {...otherProps} className={classNames(underlayStyles, 'spectrum-Underlay', {'is-open': isOpen, 'spectrum-Underlay--transparent': isTransparent})} />
2525
);
2626
}

0 commit comments

Comments
 (0)