diff --git a/packages/@react-aria/interactions/src/useInteractOutside.ts b/packages/@react-aria/interactions/src/useInteractOutside.ts index d2eec8b1d90..47cfca88a2c 100644 --- a/packages/@react-aria/interactions/src/useInteractOutside.ts +++ b/packages/@react-aria/interactions/src/useInteractOutside.ts @@ -37,6 +37,10 @@ export function useInteractOutside(props: InteractOutsideProps) { let state = stateRef.current; useEffect(() => { + if (!onInteractOutside) { + return; + } + let onPointerDown = (e) => { if (isDisabled) { return; @@ -45,18 +49,17 @@ export function useInteractOutside(props: InteractOutsideProps) { state.isPointerDown = true; } }; - /* + // FF bug https://bugzilla.mozilla.org/show_bug.cgi?id=1675846 prevents us from using this pointerevent // once it's fixed we can uncomment // Use pointer events if available. Otherwise, fall back to mouse and touch events. - if (typeof PointerEvent !== 'undefined') { + /* if (typeof PointerEvent !== 'undefined') { let onPointerUp = (e) => { if (state.isPointerDown && onInteractOutside && isValidEvent(e, ref)) { state.isPointerDown = false; onInteractOutside(e); } }; - // changing these to capture phase fixed combobox document.addEventListener('pointerdown', onPointerDown, true); document.addEventListener('pointerup', onPointerUp, true); @@ -78,6 +81,7 @@ export function useInteractOutside(props: InteractOutsideProps) { } }; + let onTouchEnd = (e) => { if (isDisabled) { return; @@ -100,6 +104,7 @@ export function useInteractOutside(props: InteractOutsideProps) { document.removeEventListener('touchstart', onPointerDown, true); document.removeEventListener('touchend', onTouchEnd, true); }; + // } }, [onInteractOutside, ref, state.ignoreEmulatedMouseEvents, state.isPointerDown, isDisabled]); } diff --git a/packages/@react-aria/interactions/test/useInteractOutside.test.js b/packages/@react-aria/interactions/test/useInteractOutside.test.js index 31747eaa0ea..007d7df18de 100644 --- a/packages/@react-aria/interactions/test/useInteractOutside.test.js +++ b/packages/@react-aria/interactions/test/useInteractOutside.test.js @@ -33,6 +33,7 @@ describe('useInteractOutside', function () { describe('pointer events', function () { installPointerEvent(); + /* TODO enable these ones pointer events are restored to useInteractOutside it('should fire interact outside events based on pointer events', function () { let onInteractOutside = jest.fn(); let res = render( @@ -40,12 +41,12 @@ describe('useInteractOutside', function () { ); let el = res.getByText('test'); - fireEvent(el, pointerEvent('mousedown')); - fireEvent(el, pointerEvent('mouseup')); + fireEvent(el, pointerEvent('pointerdown')); + fireEvent(el, pointerEvent('pointerup')); expect(onInteractOutside).not.toHaveBeenCalled(); - fireEvent(document.body, pointerEvent('mousedown')); - fireEvent(document.body, pointerEvent('mouseup')); + fireEvent(document.body, pointerEvent('pointerdown')); + fireEvent(document.body, pointerEvent('pointerup')); expect(onInteractOutside).toHaveBeenCalledTimes(1); }); @@ -55,14 +56,14 @@ describe('useInteractOutside', function () { ); - fireEvent(document.body, pointerEvent('mousedown', {button: 1})); - fireEvent(document.body, pointerEvent('mouseup', {button: 1})); + fireEvent(document.body, pointerEvent('pointerdown', {button: 1})); + fireEvent(document.body, pointerEvent('pointerup', {button: 1})); expect(onInteractOutside).not.toHaveBeenCalled(); - fireEvent(document.body, pointerEvent('mousedown', {button: 0})); - fireEvent(document.body, pointerEvent('mouseup')); + fireEvent(document.body, pointerEvent('pointerdown', {button: 0})); + fireEvent(document.body, pointerEvent('pointerup', {button: 0})); expect(onInteractOutside).toHaveBeenCalledTimes(1); - }); + });*/ it('should not fire interact outside if there is a pointer up event without a pointer down first', function () { // Fire pointer down before component with useInteractOutside is mounted diff --git a/packages/@react-aria/menu/stories/useMenuTrigger.stories.tsx b/packages/@react-aria/menu/stories/useMenuTrigger.stories.tsx new file mode 100644 index 00000000000..47cba2806df --- /dev/null +++ b/packages/@react-aria/menu/stories/useMenuTrigger.stories.tsx @@ -0,0 +1,180 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {DismissButton, useOverlay} from '@react-aria/overlays'; +import {Flex} from '@react-spectrum/layout'; +import {FocusScope} from '@react-aria/focus'; +import {Item} from '@react-stately/collections'; +import {mergeProps} from '@react-aria/utils'; +import React from 'react'; +import {storiesOf} from '@storybook/react'; +import {useButton} from '@react-aria/button'; +import {useFocus} from '@react-aria/interactions'; +import {useMenu, useMenuItem, useMenuTrigger} from '@react-aria/menu'; +import {useMenuTriggerState} from '@react-stately/menu'; +import {useTreeState} from '@react-stately/tree'; + + +storiesOf('useMenuTrigger', module) + .add('2 menus', () => ( + + + Copy + Cut + Paste + + + Copy + Cut + Paste + + + )) + .add('2 menus with disabled', () => ( + + + Copy + Cut + Paste + + + Copy + Cut + Paste + + + )); + +function MenuButton(props) { + // Create state based on the incoming props + let state = useMenuTriggerState(props); + + // Get props for the menu trigger and menu elements + let ref = React.useRef(null); + + let shouldCloseOnInteractOutside = (element) => !ref?.current?.contains(element) ?? false; + let {menuTriggerProps, menuProps} = useMenuTrigger({}, state, ref); + + // Get props for the button based on the trigger props from useMenuTrigger + let {buttonProps} = useButton({...menuTriggerProps, isDisabled: props.isDisabled}, ref); + + return ( +
+ + {state.isOpen && ( + state.close()} /> + )} +
+ ); +} + +function MenuPopup(props) { + // Create menu state based on the incoming props + let state = useTreeState({...props, selectionMode: 'none'}); + + // Get props for the menu element + let ref = React.useRef(); + let {menuProps} = useMenu(props, state, ref); + + // Handle events that should cause the menu to close, + // e.g. blur, clicking outside, or pressing the escape key. + let overlayRef = React.useRef(); + let {overlayProps} = useOverlay( + { + shouldCloseOnInteractOutside: props.shouldCloseOnInteractOutside, + onClose: props.onClose, + shouldCloseOnBlur: true, + isOpen: true, + isDismissable: true + }, + overlayRef + ); + + // Wrap in so that focus is restored back to the + // trigger when the menu is closed. In addition, add hidden + // components at the start and end of the list + // to allow screen reader users to dismiss the popup easily. + return ( + +
+ +
    + {[...state.collection].map((item) => ( + + ))} +
+ +
+ + ); +} + +function MenuItem({item, state, onAction, onClose}) { + // Get props for the menu item element + let ref = React.useRef(); + let {menuItemProps} = useMenuItem( + { + key: item.key, + isDisabled: item.isDisabled, + onAction, + onClose + }, + state, + ref + ); + + // Handle focus events so we can apply highlighted + // style to the focused menu item + let [isFocused, setFocused] = React.useState(false); + let {focusProps} = useFocus({onFocusChange: setFocused}); + + return ( +
  • + {item.rendered} +
    • + ); +} diff --git a/packages/@react-aria/overlays/src/useOverlay.ts b/packages/@react-aria/overlays/src/useOverlay.ts index c37807f4665..b6a57cfddac 100644 --- a/packages/@react-aria/overlays/src/useOverlay.ts +++ b/packages/@react-aria/overlays/src/useOverlay.ts @@ -49,7 +49,12 @@ interface OverlayAria { overlayProps: HTMLAttributes } -const visibleOverlays: RefObject[] = []; +interface OpenOverlay { + ref: RefObject, + onClose: () => void +} + +export const visibleOverlays: OpenOverlay[] = []; /** * Provides the behavior for overlays such as dialogs, popovers, and menus. @@ -62,11 +67,11 @@ export function useOverlay(props: OverlayProps, ref: RefObject): Ov // Add the overlay ref to the stack of visible overlays on mount, and remove on unmount. useEffect(() => { if (isOpen) { - visibleOverlays.push(ref); + visibleOverlays.push({ref, onClose}); } return () => { - let index = visibleOverlays.indexOf(ref); + let index = visibleOverlays.findIndex(({ref: openRef}) => ref === openRef); if (index >= 0) { visibleOverlays.splice(index, 1); } @@ -75,7 +80,7 @@ export function useOverlay(props: OverlayProps, ref: RefObject): Ov // Only hide the overlay when it is the topmost visible overlay in the stack. let onHide = () => { - if (visibleOverlays[visibleOverlays.length - 1] === ref && onClose) { + if (visibleOverlays[visibleOverlays.length - 1].ref === ref && onClose) { onClose(); } }; @@ -95,7 +100,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/overlays/src/useOverlayTrigger.ts b/packages/@react-aria/overlays/src/useOverlayTrigger.ts index 93513fd315f..78279a93028 100644 --- a/packages/@react-aria/overlays/src/useOverlayTrigger.ts +++ b/packages/@react-aria/overlays/src/useOverlayTrigger.ts @@ -15,6 +15,7 @@ import {HTMLAttributes, RefObject, useEffect} from 'react'; import {onCloseMap} from './useCloseOnScroll'; import {OverlayTriggerState} from '@react-stately/overlays'; import {useId} from '@react-aria/utils'; +import {visibleOverlays} from './useOverlay'; interface OverlayTriggerProps { /** Type of overlay that is opened by the trigger. */ @@ -45,6 +46,26 @@ export function useOverlayTrigger(props: OverlayTriggerProps, state: OverlayTrig } }); + useEffect(() => { + if (isOpen === true && visibleOverlays.length > 1) { + // The last overlay is the one just opened. + // If we have two overlays open, then we need to determine if we're nested or not. + // Start from top of the stack (minus the one we just opened) and close it if it doesn't + // contain the trigger that opened the most recent overlay. + // Do this until we find one that does contain it or close everything. + let i = visibleOverlays.length - 2; + do { + let {ref: overlayRef, onClose} = visibleOverlays[i]; + if (!overlayRef.current.contains(ref.current)) { + onClose(); + } else { + break; + } + i--; + } while (i >= 0); + } + }, [isOpen]); + // Aria 1.1 supports multiple values for aria-haspopup other than just menus. // https://www.w3.org/TR/wai-aria-1.1/#aria-haspopup // However, we only add it for menus for now because screen readers often diff --git a/packages/@react-spectrum/combobox/stories/ComboBox.stories.tsx b/packages/@react-spectrum/combobox/stories/ComboBox.stories.tsx index e138f9f04dc..ac901966497 100644 --- a/packages/@react-spectrum/combobox/stories/ComboBox.stories.tsx +++ b/packages/@react-spectrum/combobox/stories/ComboBox.stories.tsx @@ -471,6 +471,19 @@ storiesOf('ComboBox', module) () => ( ) + ) + .add( + '2 comboboxes', + () => ( + + + {(item) => {item.name}} + + + {(item) => {item.name}} + + + ) ); function LoadingExamples(props) { diff --git a/packages/@react-spectrum/dialog/src/DialogTrigger.tsx b/packages/@react-spectrum/dialog/src/DialogTrigger.tsx index 5246d2f1782..283950020d2 100644 --- a/packages/@react-spectrum/dialog/src/DialogTrigger.tsx +++ b/packages/@react-spectrum/dialog/src/DialogTrigger.tsx @@ -163,6 +163,7 @@ function PopoverTrigger({state, targetRef, trigger, content, hideArrow, isKeyboa ...triggerProps, ref: targetRef ? undefined : triggerRef }; + let shouldCloseOnInteractOutside = (element) => !triggerRef.current?.contains(element); let overlay = ( {typeof content === 'function' ? content(state.close) : content} diff --git a/packages/@react-spectrum/dialog/stories/DialogTrigger.stories.tsx b/packages/@react-spectrum/dialog/stories/DialogTrigger.stories.tsx index e09dbb74fbd..ce773b5c726 100644 --- a/packages/@react-spectrum/dialog/stories/DialogTrigger.stories.tsx +++ b/packages/@react-spectrum/dialog/stories/DialogTrigger.stories.tsx @@ -331,6 +331,26 @@ storiesOf('DialogTrigger', module) ) + ) + .add( + '2 popovers', + () => ( + + + Trigger + + + + + + + + + Trigger + Hi! + + + ) ); function render({width = 'auto', ...props}) { diff --git a/packages/@react-spectrum/menu/src/MenuTrigger.tsx b/packages/@react-spectrum/menu/src/MenuTrigger.tsx index b836ff1c692..6f388611f9d 100644 --- a/packages/@react-spectrum/menu/src/MenuTrigger.tsx +++ b/packages/@react-spectrum/menu/src/MenuTrigger.tsx @@ -74,6 +74,7 @@ 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; @@ -92,7 +93,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 5457808766e..ca0861ec2d9 100644 --- a/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx +++ b/packages/@react-spectrum/menu/stories/MenuTrigger.stories.tsx @@ -19,10 +19,11 @@ 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'; -import React from 'react'; +import React, {useState} from 'react'; import {storiesOf} from '@storybook/react'; let iconMap = { @@ -487,7 +488,45 @@ storiesOf('MenuTrigger', module) Three ) + ) + .add('double menu', () => ( + + + + Menu Button 1 + + {defaultMenu} + + + + Menu Button 2 + + {defaultMenu} + + + )) + .add('double menu controlled', () => ); + +function DoubleMenuControlled() { + let [isOpen1, setIsOpen1] = useState(true); + let [isOpen2, setIsOpen2] = useState(true); + return ( + + + + 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 cdc4c1f0770..1cac888a382 100644 --- a/packages/@react-spectrum/menu/test/MenuTrigger.test.js +++ b/packages/@react-spectrum/menu/test/MenuTrigger.test.js @@ -14,7 +14,7 @@ import {act, fireEvent, render, within} from '@testing-library/react'; import {Button} from '@react-spectrum/button'; import {Item, Menu, MenuTrigger, Section} from '../'; import {Provider} from '@react-spectrum/provider'; -import React from 'react'; +import React, {useState} from 'react'; import {theme} from '@react-spectrum/theme-default'; import {triggerPress} from '@react-spectrum/test-utils'; import V2Button from '@react/react-spectrum/Button'; @@ -222,7 +222,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 @@ -759,4 +759,129 @@ describe('MenuTrigger', function () { let checkmark = queryByRole('img', {hidden: true}); expect(checkmark).toBeNull(); }); + + describe('2 menus', function () { + it('should close the first when the second opens', function () { + let onOpenChange1 = jest.fn(); + let onOpenChange2 = jest.fn(); + let {queryByTestId} = render( + <> + + + + + {item => ( +
    + {item => {item.name}} +
    + )} +     +     + + + + {item => ( +
    + {item => {item.name}} +
    + )} +     +     +     + + ); + act(() => {jest.runAllTimers();}); + let menu1 = queryByTestId('menu1'); + expect(onOpenChange1).not.toHaveBeenCalled(); // its "already" open + expect(onOpenChange1).toHaveBeenCalledTimes(0); + expect(onOpenChange2).not.toHaveBeenCalled(); + expect(onOpenChange2).toHaveBeenCalledTimes(0); + expect(menu1).toBeVisible(); + let button2 = queryByTestId('button2'); + triggerPress(button2); + act(() => {jest.runAllTimers();}); + expect(onOpenChange1).toHaveBeenCalledWith(false); + expect(onOpenChange1).toHaveBeenCalledTimes(1); + expect(onOpenChange2).toHaveBeenCalledWith(true); + expect(onOpenChange2).toHaveBeenCalledTimes(1); + let menu2 = queryByTestId('menu2'); + expect(menu1).not.toBeInTheDocument(); + expect(menu2).toBeVisible(); + }); + + it('should close the first when the second opens if two are opened at once', function () { + let {queryByTestId} = render( + <> + + + + + {item => ( +
    + {item => {item.name}} +
    + )} +     +     + + + + {item => ( +
    + {item => {item.name}} +
    + )} +     +     +     + + ); + act(() => {jest.runAllTimers();}); + let menu1 = queryByTestId('menu1'); + expect(menu1).toBeNull(); + let menu2 = queryByTestId('menu2'); + expect(menu2).toBeVisible(); + }); + + it('should work in the controlled case as well', function () { + function ControlledMenu(props) { + let [isOpen, setIsOpen] = useState(true); + return ( + + + + {item => ( +
    + {item => {item.name}} +
    + )} +     +     + ); + } + let {queryByTestId} = render( + <> + + + + + + ); + act(() => {jest.runAllTimers();}); + let menu1 = queryByTestId('menu1'); + expect(menu1).toBeNull(); + let menu2 = queryByTestId('menu2'); + expect(menu2).toBeVisible(); + }); + }); }); diff --git a/packages/@react-spectrum/overlays/src/Popover.tsx b/packages/@react-spectrum/overlays/src/Popover.tsx index ac6a0e8678c..7ad56d64ae4 100644 --- a/packages/@react-spectrum/overlays/src/Popover.tsx +++ b/packages/@react-spectrum/overlays/src/Popover.tsx @@ -29,7 +29,8 @@ interface PopoverWrapperProps extends HTMLAttributes { onClose?: () => void, shouldCloseOnBlur?: boolean, isKeyboardDismissDisabled?: boolean, - isNonModal?: boolean + isNonModal?: boolean, + shouldCloseOnInteractOutside?: (element: HTMLElement) => boolean } /** @@ -47,7 +48,7 @@ let arrowPlacement = { }; function Popover(props: PopoverProps, ref: DOMRef) { - let {children, placement, arrowProps, onClose, shouldCloseOnBlur, hideArrow, isKeyboardDismissDisabled, isNonModal, ...otherProps} = props; + let {children, placement, arrowProps, onClose, shouldCloseOnBlur, shouldCloseOnInteractOutside, hideArrow, isKeyboardDismissDisabled, isNonModal, ...otherProps} = props; let domRef = useDOMRef(ref); let {styleProps} = useStyleProps(props); @@ -61,6 +62,7 @@ function Popover(props: PopoverProps, ref: DOMRef) { onClose={onClose} shouldCloseOnBlur={shouldCloseOnBlur} isKeyboardDismissDisabled={isKeyboardDismissDisabled} + shouldCloseOnInteractOutside={shouldCloseOnInteractOutside} hideArrow={hideArrow} isNonModal={isNonModal}> {children} @@ -71,7 +73,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, isNonModal, ...otherProps} = props; + let {children, placement = 'bottom', arrowProps, isOpen, hideArrow, shouldCloseOnBlur, shouldCloseOnInteractOutside, isKeyboardDismissDisabled, isNonModal, ...otherProps} = props; let {overlayProps} = useOverlay({...props, isDismissable: true}, ref); let {modalProps} = useModal({ isDisabled: isNonModal diff --git a/packages/@react-spectrum/picker/src/Picker.tsx b/packages/@react-spectrum/picker/src/Picker.tsx index 373ec050fed..88c2bdfdc2b 100644 --- a/packages/@react-spectrum/picker/src/Picker.tsx +++ b/packages/@react-spectrum/picker/src/Picker.tsx @@ -170,6 +170,7 @@ function Picker(props: SpectrumPickerProps, ref: DOMRef !triggerRef.current?.UNSAFE_getDOMNode()?.contains(element); overlay = ( (props: SpectrumPickerProps, ref: DOMRef {listbox} diff --git a/packages/@react-spectrum/picker/stories/Picker.stories.tsx b/packages/@react-spectrum/picker/stories/Picker.stories.tsx index 03734ffa80b..da680aafa62 100644 --- a/packages/@react-spectrum/picker/stories/Picker.stories.tsx +++ b/packages/@react-spectrum/picker/stories/Picker.stories.tsx @@ -532,17 +532,37 @@ storiesOf('Picker', module) Three )) - .add('scrolling container', () => ( - - - + .add( + '2 pickers', + () => ( + + One Two Three + + One + Two + Three + + + ) + ) + .add( + 'scrolling container', + () => ( + + + + One + Two + Three + + - - )); + ) + ); function AsyncLoadingExample() { interface Pokemon { diff --git a/packages/@react-stately/overlays/src/useOverlayTriggerState.ts b/packages/@react-stately/overlays/src/useOverlayTriggerState.ts index 8e1f7c2b28c..a806904f079 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(val => !val); } }; } diff --git a/packages/@react-types/overlays/src/index.d.ts b/packages/@react-types/overlays/src/index.d.ts index e149bf73561..db92951d83a 100644 --- a/packages/@react-types/overlays/src/index.d.ts +++ b/packages/@react-types/overlays/src/index.d.ts @@ -90,7 +90,8 @@ export interface PopoverProps extends StyleProps, OverlayProps { isOpen?: boolean, onClose?: () => void, shouldCloseOnBlur?: boolean, - isNonModal?: boolean + isNonModal?: boolean, + shouldCloseOnInteractOutside?: (element: HTMLElement) => boolean } export interface TrayProps extends StyleProps, OverlayProps {