Skip to content
11 changes: 8 additions & 3 deletions packages/@react-aria/interactions/src/useInteractOutside.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export function useInteractOutside(props: InteractOutsideProps) {
let state = stateRef.current;

useEffect(() => {
if (!onInteractOutside) {
return;
}

let onPointerDown = (e) => {
if (isDisabled) {
return;
Expand All @@ -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);
Expand All @@ -78,6 +81,7 @@ export function useInteractOutside(props: InteractOutsideProps) {
}
};


let onTouchEnd = (e) => {
if (isDisabled) {
return;
Expand All @@ -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]);
}

Expand Down
19 changes: 10 additions & 9 deletions packages/@react-aria/interactions/test/useInteractOutside.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,20 @@ 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(
<Example onInteractOutside={onInteractOutside} />
);

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);
});

Expand All @@ -55,14 +56,14 @@ describe('useInteractOutside', function () {
<Example onInteractOutside={onInteractOutside} />
);

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
Expand Down
180 changes: 180 additions & 0 deletions packages/@react-aria/menu/stories/useMenuTrigger.stories.tsx
Original file line number Diff line number Diff line change
@@ -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', () => (
<Flex>
<MenuButton label="Actions">
<Item key="copy">Copy</Item>
<Item key="cut">Cut</Item>
<Item key="paste">Paste</Item>
</MenuButton>
<MenuButton label="Actions2">
<Item key="copy">Copy</Item>
<Item key="cut">Cut</Item>
<Item key="paste">Paste</Item>
</MenuButton>
</Flex>
))
.add('2 menus with disabled', () => (
<Flex>
<MenuButton isDisabled label="Actions">
<Item key="copy">Copy</Item>
<Item key="cut">Cut</Item>
<Item key="paste">Paste</Item>
</MenuButton>
<MenuButton label="Actions2">
<Item key="copy">Copy</Item>
<Item key="cut">Cut</Item>
<Item key="paste">Paste</Item>
</MenuButton>
</Flex>
));

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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this prop needed? Would be nice to avoid making everyone writing an overlay pass it in...

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 (
<div style={{position: 'relative', display: 'inline-block'}}>
<button {...buttonProps} disabled={props.isDisabled} ref={ref} style={{height: 30, fontSize: 14}}>
{props.label}
<span aria-hidden="true" style={{paddingLeft: 5}}>
</span>
</button>
{state.isOpen && (
<MenuPopup
{...props}
shouldCloseOnInteractOutside={shouldCloseOnInteractOutside}
domProps={menuProps}
autoFocus={state.focusStrategy}
onClose={() => state.close()} />
)}
</div>
);
}

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 <FocusScope> so that focus is restored back to the
// trigger when the menu is closed. In addition, add hidden
// <DismissButton> components at the start and end of the list
// to allow screen reader users to dismiss the popup easily.
return (
<FocusScope restoreFocus>
<div {...overlayProps} ref={overlayRef}>
<DismissButton onDismiss={props.onClose} />
<ul
{...mergeProps(menuProps, props.domProps)}
ref={ref}
style={{
position: 'absolute',
width: '100%',
margin: '4px 0 0 0',
padding: 0,
listStyle: 'none',
border: '1px solid gray',
background: 'lightgray'
}}>
{[...state.collection].map((item) => (
<MenuItem
key={item.key}
item={item}
state={state}
onAction={props.onAction}
onClose={props.onClose} />
))}
</ul>
<DismissButton onDismiss={props.onClose} />
</div>
</FocusScope>
);
}

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 (
<li
{...mergeProps(menuItemProps, focusProps)}
ref={ref}
style={{
background: isFocused ? 'gray' : 'transparent',
color: isFocused ? 'white' : 'black',
padding: '2px 5px',
outline: 'none',
cursor: 'pointer'
}}>
{item.rendered}
</li>
);
}
15 changes: 10 additions & 5 deletions packages/@react-aria/overlays/src/useOverlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,12 @@ interface OverlayAria {
overlayProps: HTMLAttributes<HTMLElement>
}

const visibleOverlays: RefObject<HTMLElement>[] = [];
interface OpenOverlay {
ref: RefObject<HTMLElement>,
onClose: () => void
}

export const visibleOverlays: OpenOverlay[] = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make sure that this isn't exported from the package (e.g. index.ts).


/**
* Provides the behavior for overlays such as dialogs, popovers, and menus.
Expand All @@ -62,11 +67,11 @@ export function useOverlay(props: OverlayProps, ref: RefObject<HTMLElement>): 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);
}
Expand All @@ -75,7 +80,7 @@ export function useOverlay(props: OverlayProps, ref: RefObject<HTMLElement>): 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();
}
};
Expand All @@ -95,7 +100,7 @@ export function useOverlay(props: OverlayProps, ref: RefObject<HTMLElement>): 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,
Expand Down
21 changes: 21 additions & 0 deletions packages/@react-aria/overlays/src/useOverlayTrigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -45,6 +46,26 @@ export function useOverlayTrigger(props: OverlayTriggerProps, state: OverlayTrig
}
});

useEffect(() => {
if (isOpen === true && visibleOverlays.length > 1) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devongovett yep, I think I had it working for one, then changed for the other and forgot to retest. I can reproduce it and I can fix it for one or the other. I'm having trouble solving for both though. It's this line here
having it as 1 solves it for the hooks which have no delay
having it as 0 solves it for the components which have the OpenTransition with a delay of 0 (which gets queued)

without knowing the overlay this trigger is opening, it's hard to determine if it's in the visibleOverlays or not yet

Some thoughts on it so far:
I could pass in overlayRef, if it's undefined then i know we're dealing with the delayed case, but that changes the API unless I make it optional and then because it's checking undefined it's hard to know if it's omitted or just that it's not mounted yet.

We can't check the id of the last visible overlay, the id isn't on the ref, it's somewhere further down in the tree.

We can't pass the id into useOverlay for storage in the array of visibleOverlays because it's on the dialog or menu context, so we'd need to change an API to get it to the useOverlay hook.

I thought about waiting to remove previous overlays dependent on when the new overlay mounts, but if there's a delay bigger than 0 to show it, then that might get a little funny looking? I think it'd be messy to implement as it'd be a new piece of state i think.

// 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
Expand Down
13 changes: 13 additions & 0 deletions packages/@react-spectrum/combobox/stories/ComboBox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,19 @@ storiesOf('ComboBox', module)
() => (
<AsyncLoadingExample />
)
)
.add(
'2 comboboxes',
() => (
<Flex gap="size-100">
<ComboBox defaultItems={items} label="Combobox1">
{(item) => <Item>{item.name}</Item>}
</ComboBox>
<ComboBox defaultItems={items} label="Combobox2">
{(item) => <Item>{item.name}</Item>}
</ComboBox>
</Flex>
)
);

function LoadingExamples(props) {
Expand Down
2 changes: 2 additions & 0 deletions packages/@react-spectrum/dialog/src/DialogTrigger.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
<Popover
Expand All @@ -172,6 +173,7 @@ function PopoverTrigger({state, targetRef, trigger, content, hideArrow, isKeyboa
onClose={state.close}
placement={placement}
arrowProps={arrowProps}
shouldCloseOnInteractOutside={shouldCloseOnInteractOutside}
isKeyboardDismissDisabled={isKeyboardDismissDisabled}
hideArrow={hideArrow}>
{typeof content === 'function' ? content(state.close) : content}
Expand Down
Loading