diff --git a/packages/@react-aria/interactions/src/useInteractOutside.ts b/packages/@react-aria/interactions/src/useInteractOutside.ts index 5c91732d9b5..5ba764857d1 100644 --- a/packages/@react-aria/interactions/src/useInteractOutside.ts +++ b/packages/@react-aria/interactions/src/useInteractOutside.ts @@ -29,65 +29,48 @@ interface InteractOutsideProps { export function useInteractOutside(props: InteractOutsideProps) { let {ref, onInteractOutside} = props; let stateRef = useRef({ - isPointerDown: false, ignoreEmulatedMouseEvents: false }); let state = stateRef.current; useEffect(() => { + if (!onInteractOutside) { + return; + } let onPointerDown = (e) => { + if (state.ignoreEmulatedMouseEvents) { + state.ignoreEmulatedMouseEvents = false; + return; + } if (isValidEvent(e, ref)) { - state.isPointerDown = true; + onInteractOutside(e); } }; // Use pointer events if available. Otherwise, fall back to mouse and touch events. if (typeof PointerEvent !== 'undefined') { - let onPointerUp = (e) => { - if (state.isPointerDown && onInteractOutside && isValidEvent(e, ref)) { - state.isPointerDown = false; - onInteractOutside(e); - } - }; - document.addEventListener('pointerdown', onPointerDown, false); - document.addEventListener('pointerup', onPointerUp, false); return () => { document.removeEventListener('pointerdown', onPointerDown, false); - document.removeEventListener('pointerup', onPointerUp, false); }; } else { - let onMouseUp = (e) => { - if (state.ignoreEmulatedMouseEvents) { - state.ignoreEmulatedMouseEvents = false; - } else if (state.isPointerDown && onInteractOutside && isValidEvent(e, ref)) { - state.isPointerDown = false; - onInteractOutside(e); - } - }; - - let onTouchEnd = (e) => { - state.ignoreEmulatedMouseEvents = true; - if (onInteractOutside && state.isPointerDown && isValidEvent(e, ref)) { - state.isPointerDown = false; + let onTouchStart = (e) => { + if (isValidEvent(e, ref)) { + state.ignoreEmulatedMouseEvents = true; onInteractOutside(e); } }; document.addEventListener('mousedown', onPointerDown, false); - document.addEventListener('mouseup', onMouseUp, false); - document.addEventListener('touchstart', onPointerDown, false); - document.addEventListener('touchend', onTouchEnd, false); + document.addEventListener('touchstart', onTouchStart, false); return () => { document.removeEventListener('mousedown', onPointerDown, false); - document.removeEventListener('mouseup', onMouseUp, false); document.removeEventListener('touchstart', onPointerDown, false); - document.removeEventListener('touchend', onTouchEnd, false); }; } - }, [onInteractOutside, ref, state.ignoreEmulatedMouseEvents, state.isPointerDown]); + }, [onInteractOutside, ref, state]); } function isValidEvent(event, ref) { diff --git a/packages/@react-aria/overlays/src/useOverlay.ts b/packages/@react-aria/overlays/src/useOverlay.ts index b0a431087ad..951a187abdb 100644 --- a/packages/@react-aria/overlays/src/useOverlay.ts +++ b/packages/@react-aria/overlays/src/useOverlay.ts @@ -29,8 +29,8 @@ interface OverlayProps { /** Whether the overlay should close when focus is lost or moves outside it. */ shouldCloseOnBlur?: boolean, - /** - * Whether pressing the escape key to close the overlay should be disabled. + /** + * Whether pressing the escape key to close the overlay should be disabled. * @default false */ isKeyboardDismissDisabled?: boolean, @@ -38,7 +38,7 @@ interface OverlayProps { /** * When user interacts with the argument element outside of the overlay ref, * return true if onClose should be called. This gives you a chance to filter - * out interaction with elements that should not dismiss the overlay. + * out interaction with elements that should not dismiss the overlay. * By default, onClose will always be called on interaction outside the overlay ref. */ shouldCloseOnInteractOutside?: (element: HTMLElement) => boolean @@ -95,7 +95,7 @@ export function useOverlay(props: OverlayProps, ref: RefObject): Ov }; // Handle clicking outside the overlay to close it - useInteractOutside({ref, onInteractOutside: isDismissable ? onInteractOutside : null}); + useInteractOutside({ref, onInteractOutside: isDismissable && isOpen ? onInteractOutside : undefined}); let {focusWithinProps} = useFocusWithin({ isDisabled: !shouldCloseOnBlur, diff --git a/packages/@react-aria/select/src/useSelect.ts b/packages/@react-aria/select/src/useSelect.ts index 8220d5696ad..ea286cf50c5 100644 --- a/packages/@react-aria/select/src/useSelect.ts +++ b/packages/@react-aria/select/src/useSelect.ts @@ -81,7 +81,7 @@ export function useSelect(props: AriaSelectOptions, state: SelectState, }); let domProps = filterDOMProps(props, {labelable: true}); - let triggerProps = mergeProps(mergeProps(menuTriggerProps, fieldProps), typeSelectProps); + let triggerProps = mergeProps(menuTriggerProps, fieldProps, typeSelectProps); let valueId = useId(); return { diff --git a/packages/@react-spectrum/menu/src/MenuTrigger.tsx b/packages/@react-spectrum/menu/src/MenuTrigger.tsx index 7169d013599..29900a5011e 100644 --- a/packages/@react-spectrum/menu/src/MenuTrigger.tsx +++ b/packages/@react-spectrum/menu/src/MenuTrigger.tsx @@ -73,6 +73,8 @@ function MenuTrigger(props: SpectrumMenuTriggerProps, ref: DOMRef) ); + let shouldCloseOnInteractOutside = (element) => !menuTriggerRef.current?.contains(element); + // On small screen devices, the menu is rendered in a tray, otherwise a popover. let overlay; if (isMobile) { @@ -90,7 +92,8 @@ function MenuTrigger(props: SpectrumMenuTriggerProps, ref: DOMRef) placement={placement} hideArrow onClose={state.close} - shouldCloseOnBlur> + shouldCloseOnBlur + shouldCloseOnInteractOutside={shouldCloseOnInteractOutside}> {contents} ); diff --git a/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx b/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx index 8cbbabce0f5..4873211ea55 100644 --- a/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx +++ b/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx @@ -19,6 +19,7 @@ import Blower from '@spectrum-icons/workflow/Blower'; import Book from '@spectrum-icons/workflow/Book'; import Copy from '@spectrum-icons/workflow/Copy'; import Cut from '@spectrum-icons/workflow/Cut'; +import {Flex} from '@react-spectrum/layout'; import {Item, Menu, MenuTrigger, Section} from '../'; import {Keyboard, Text} from '@react-spectrum/text'; import Paste from '@spectrum-icons/workflow/Paste'; @@ -477,7 +478,30 @@ storiesOf('MenuTrigger', module) ) - ); + ) + .add('double menu', () => ( + + + + Menu Button 1 + + {defaultMenu} + + + + Menu Button 2 + + {defaultMenu} + + + )); + let customMenuItem = (item) => { let Icon = iconMap[item.icon]; diff --git a/packages/@react-spectrum/menu/test/MenuTrigger.test.js b/packages/@react-spectrum/menu/test/MenuTrigger.test.js index 75774af956b..62db5a41b5a 100644 --- a/packages/@react-spectrum/menu/test/MenuTrigger.test.js +++ b/packages/@react-spectrum/menu/test/MenuTrigger.test.js @@ -221,7 +221,7 @@ describe('MenuTrigger', function () { menu = tree.getByRole('menu'); expect(menu).toBeTruthy(); - expect(onOpenChange).toBeCalledTimes(2); // once for press, once for blur :/ + expect(onOpenChange).toBeCalledTimes(1); }); // New functionality in v3 diff --git a/packages/@react-spectrum/overlays/src/Popover.tsx b/packages/@react-spectrum/overlays/src/Popover.tsx index 56c9d5b8cd8..28951316205 100644 --- a/packages/@react-spectrum/overlays/src/Popover.tsx +++ b/packages/@react-spectrum/overlays/src/Popover.tsx @@ -28,7 +28,8 @@ interface PopoverWrapperProps extends HTMLAttributes { isOpen?: boolean, onClose?: () => void, shouldCloseOnBlur?: boolean, - isKeyboardDismissDisabled?: boolean + isKeyboardDismissDisabled?: boolean, + shouldCloseOnInteractOutside?: (element: HTMLElement) => boolean } /** @@ -46,7 +47,7 @@ let arrowPlacement = { }; function Popover(props: PopoverProps, ref: DOMRef) { - let {children, placement, arrowProps, onClose, shouldCloseOnBlur, hideArrow, isKeyboardDismissDisabled, ...otherProps} = props; + let {children, placement, arrowProps, onClose, shouldCloseOnBlur, hideArrow, isKeyboardDismissDisabled, shouldCloseOnInteractOutside, ...otherProps} = props; let domRef = useDOMRef(ref); let {styleProps} = useStyleProps(props); @@ -60,7 +61,8 @@ function Popover(props: PopoverProps, ref: DOMRef) { onClose={onClose} shouldCloseOnBlur={shouldCloseOnBlur} isKeyboardDismissDisabled={isKeyboardDismissDisabled} - hideArrow={hideArrow}> + hideArrow={hideArrow} + shouldCloseOnInteractOutside={shouldCloseOnInteractOutside}> {children} @@ -69,7 +71,7 @@ function Popover(props: PopoverProps, ref: DOMRef) { const PopoverWrapper = forwardRef((props: PopoverWrapperProps, ref: RefObject) => { // eslint-disable-next-line @typescript-eslint/no-unused-vars - let {children, placement = 'bottom', arrowProps, isOpen, hideArrow, shouldCloseOnBlur, isKeyboardDismissDisabled, ...otherProps} = props; + let {children, placement = 'bottom', arrowProps, isOpen, hideArrow, shouldCloseOnBlur, isKeyboardDismissDisabled, shouldCloseOnInteractOutside, ...otherProps} = props; let {overlayProps} = useOverlay({...props, isDismissable: true}, ref); let {modalProps} = useModal(); diff --git a/packages/@react-spectrum/picker/src/Picker.tsx b/packages/@react-spectrum/picker/src/Picker.tsx index 1bb7a171938..b0855d7dc57 100644 --- a/packages/@react-spectrum/picker/src/Picker.tsx +++ b/packages/@react-spectrum/picker/src/Picker.tsx @@ -149,6 +149,8 @@ function Picker(props: SpectrumPickerProps, ref: DOMRef !triggerRef.current?.UNSAFE_getDOMNode()?.contains(element); + overlay = ( (props: SpectrumPickerProps, ref: DOMRef + onClose={state.close} + shouldCloseOnInteractOutside={shouldCloseOnInteractOutside}> {listbox} ); diff --git a/packages/@react-spectrum/picker/stories/Picker.stories.tsx b/packages/@react-spectrum/picker/stories/Picker.stories.tsx index ab6fed1aafd..d2895e2e2e2 100644 --- a/packages/@react-spectrum/picker/stories/Picker.stories.tsx +++ b/packages/@react-spectrum/picker/stories/Picker.stories.tsx @@ -54,7 +54,7 @@ storiesOf('Picker', module) .add( 'default', () => ( - + One Two Three diff --git a/packages/@react-spectrum/picker/test/Picker.test.js b/packages/@react-spectrum/picker/test/Picker.test.js index a937c215d8a..5c9025867d7 100644 --- a/packages/@react-spectrum/picker/test/Picker.test.js +++ b/packages/@react-spectrum/picker/test/Picker.test.js @@ -23,6 +23,7 @@ import React from 'react'; import {Text} from '@react-spectrum/text'; import {theme} from '@react-spectrum/theme-default'; import {triggerPress} from '@react-spectrum/test-utils'; +import userEvent from '@testing-library/user-event'; describe('Picker', function () { let offsetWidth, offsetHeight; @@ -365,7 +366,7 @@ describe('Picker', function () { expect(() => getByRole('listbox')).toThrow(); let picker = getByRole('button'); - act(() => triggerPress(picker)); + act(() => userEvent.click(picker)); act(() => jest.runAllTimers()); let listbox = getByRole('listbox'); @@ -375,7 +376,7 @@ describe('Picker', function () { expect(picker).toHaveAttribute('aria-expanded', 'true'); expect(picker).toHaveAttribute('aria-controls', listbox.id); - act(() => triggerPress(picker)); + act(() => userEvent.click(picker)); act(() => jest.runAllTimers()); expect(listbox).not.toBeInTheDocument(); diff --git a/packages/@react-stately/overlays/src/useOverlayTriggerState.ts b/packages/@react-stately/overlays/src/useOverlayTriggerState.ts index 8e1f7c2b28c..3ca89ded7c9 100644 --- a/packages/@react-stately/overlays/src/useOverlayTriggerState.ts +++ b/packages/@react-stately/overlays/src/useOverlayTriggerState.ts @@ -40,7 +40,7 @@ export function useOverlayTriggerState(props: OverlayTriggerProps): OverlayTrigg setOpen(false); }, toggle() { - setOpen(!isOpen); + setOpen(prev => !prev); } }; } diff --git a/packages/@react-types/overlays/src/index.d.ts b/packages/@react-types/overlays/src/index.d.ts index a8ab9e416b7..d19a7abc2fe 100644 --- a/packages/@react-types/overlays/src/index.d.ts +++ b/packages/@react-types/overlays/src/index.d.ts @@ -89,7 +89,8 @@ export interface PopoverProps extends StyleProps, OverlayProps { hideArrow?: boolean, isOpen?: boolean, onClose?: () => void, - shouldCloseOnBlur?: boolean + shouldCloseOnBlur?: boolean, + shouldCloseOnInteractOutside?: (element: HTMLElement) => boolean } export interface TrayProps extends StyleProps, OverlayProps {