-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Add Shadow DOM support for useOverlay and improve event handling #7751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
c37229e
f09384a
6c1a00e
da35a42
96a3364
e6e839c
42cc5cb
bd82fbb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ | |
*/ | ||
|
||
import {DOMAttributes, RefObject} from '@react-types/shared'; | ||
import {getEventTarget} from '@react-aria/utils'; | ||
import {isElementInChildOfActiveScope} from '@react-aria/focus'; | ||
import {useEffect} from 'react'; | ||
import {useFocusWithin, useInteractOutside} from '@react-aria/interactions'; | ||
|
@@ -92,7 +93,7 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject<Element | nul | |
}; | ||
|
||
let onInteractOutsideStart = (e: PointerEvent) => { | ||
if (!shouldCloseOnInteractOutside || shouldCloseOnInteractOutside(e.target as Element)) { | ||
if (!shouldCloseOnInteractOutside || shouldCloseOnInteractOutside(getEventTarget(e))) { | ||
if (visibleOverlays[visibleOverlays.length - 1] === ref) { | ||
e.stopPropagation(); | ||
e.preventDefault(); | ||
|
@@ -101,7 +102,7 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject<Element | nul | |
}; | ||
|
||
let onInteractOutside = (e: PointerEvent) => { | ||
if (!shouldCloseOnInteractOutside || shouldCloseOnInteractOutside(e.target as Element)) { | ||
if (!shouldCloseOnInteractOutside || shouldCloseOnInteractOutside(getEventTarget(e))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there are some 'relatedTarget' checks below, can we dive into a shadowDOM for those as well? i don't know if there is a composedPath for them I could see this being an issue if getEventTarget is contained in e.relatedTarget or if the e.relatedTarget is a shadowroot and contains both targets that should be ignored as well as ones which shouldn't. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it weird that we call an external API with something that should be inside a shadowroot, it'd unintentionally leak shadow dom internals through our API We don't do this in the pressEvents, though the use case is obviously a little different |
||
if (visibleOverlays[visibleOverlays.length - 1] === ref) { | ||
e.stopPropagation(); | ||
e.preventDefault(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,17 +10,30 @@ | |
* governing permissions and limitations under the License. | ||
*/ | ||
|
||
import {fireEvent, installMouseEvent, installPointerEvent, render} from '@react-spectrum/test-utils-internal'; | ||
import { | ||
createShadowRoot, | ||
fireEvent, | ||
installMouseEvent, | ||
installPointerEvent, | ||
render | ||
} from '@react-spectrum/test-utils-internal'; | ||
import {enableShadowDOM} from '@react-stately/flags'; | ||
import {mergeProps} from '@react-aria/utils'; | ||
import React, {useRef} from 'react'; | ||
import ReactDOM from 'react-dom'; | ||
import {useOverlay} from '../'; | ||
|
||
function Example(props) { | ||
let ref = useRef(); | ||
let {overlayProps, underlayProps} = useOverlay(props, ref); | ||
return ( | ||
<div {...mergeProps(underlayProps, props.underlayProps || {})}> | ||
<div ref={ref} {...overlayProps} data-testid={props['data-testid'] || 'test'}> | ||
<div | ||
{...mergeProps(underlayProps, props.underlayProps || {})} | ||
data-testid={'underlay'}> | ||
<div | ||
ref={ref} | ||
{...overlayProps} | ||
data-testid={props['data-testid'] || 'test'}> | ||
{props.children} | ||
</div> | ||
</div> | ||
|
@@ -29,19 +42,10 @@ function Example(props) { | |
|
||
describe('useOverlay', function () { | ||
describe.each` | ||
type | prepare | actions | ||
${'Mouse Events'} | ${installMouseEvent} | ${[ | ||
(el) => fireEvent.mouseDown(el, {button: 0}), | ||
(el) => fireEvent.mouseUp(el, {button: 0}) | ||
]} | ||
${'Pointer Events'} | ${installPointerEvent}| ${[ | ||
(el) => fireEvent.pointerDown(el, {button: 0, pointerId: 1}), | ||
(el) => fireEvent.pointerUp(el, {button: 0, pointerId: 1}) | ||
]} | ||
${'Touch Events'} | ${() => {}} | ${[ | ||
(el) => fireEvent.touchStart(el, {changedTouches: [{identifier: 1}]}), | ||
(el) => fireEvent.touchEnd(el, {changedTouches: [{identifier: 1}]}) | ||
]} | ||
type | prepare | actions | ||
ritz078 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
${'Mouse Events'} | ${installMouseEvent} | ${[(el) => fireEvent.mouseDown(el, {button: 0}), (el) => fireEvent.mouseUp(el, {button: 0})]} | ||
${'Pointer Events'} | ${installPointerEvent} | ${[(el) => fireEvent.pointerDown(el, {button: 0, pointerId: 1}), (el) => fireEvent.pointerUp(el, {button: 0, pointerId: 1})]} | ||
${'Touch Events'} | ${() => {}} | ${[(el) => fireEvent.touchStart(el, {changedTouches: [{identifier: 1}]}), (el) => fireEvent.touchEnd(el, {changedTouches: [{identifier: 1}]})]} | ||
`('$type', ({actions: [pressStart, pressEnd], prepare}) => { | ||
prepare(); | ||
|
||
|
@@ -56,7 +60,7 @@ describe('useOverlay', function () { | |
expect(document.activeElement).toBe(input); | ||
}); | ||
|
||
it('should hide the overlay when clicking outside if isDismissble is true', function () { | ||
it('should hide the overlay when clicking outside if isDismissable is true', function () { | ||
let onClose = jest.fn(); | ||
render(<Example isOpen onClose={onClose} isDismissable />); | ||
pressStart(document.body); | ||
|
@@ -66,15 +70,27 @@ describe('useOverlay', function () { | |
|
||
it('should hide the overlay when clicking outside if shouldCloseOnInteractOutside returns true', function () { | ||
let onClose = jest.fn(); | ||
render(<Example isOpen onClose={onClose} isDismissable shouldCloseOnInteractOutside={target => target === document.body} />); | ||
render( | ||
<Example | ||
isOpen | ||
onClose={onClose} | ||
isDismissable | ||
shouldCloseOnInteractOutside={(target) => target === document.body} /> | ||
); | ||
pressStart(document.body); | ||
pressEnd(document.body); | ||
expect(onClose).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it('should not hide the overlay when clicking outside if shouldCloseOnInteractOutside returns false', function () { | ||
let onClose = jest.fn(); | ||
render(<Example isOpen onClose={onClose} isDismissable shouldCloseOnInteractOutside={target => target !== document.body} />); | ||
render( | ||
<Example | ||
isOpen | ||
onClose={onClose} | ||
isDismissable | ||
shouldCloseOnInteractOutside={(target) => target !== document.body} /> | ||
); | ||
pressStart(document.body); | ||
pressEnd(document.body); | ||
expect(onClose).toHaveBeenCalledTimes(0); | ||
|
@@ -92,7 +108,9 @@ describe('useOverlay', function () { | |
let onCloseFirst = jest.fn(); | ||
let onCloseSecond = jest.fn(); | ||
render(<Example isOpen onClose={onCloseFirst} isDismissable />); | ||
let second = render(<Example isOpen onClose={onCloseSecond} isDismissable />); | ||
let second = render( | ||
<Example isOpen onClose={onCloseSecond} isDismissable /> | ||
); | ||
|
||
pressStart(document.body); | ||
pressEnd(document.body); | ||
|
@@ -117,7 +135,9 @@ describe('useOverlay', function () { | |
|
||
it('should still hide the overlay when pressing the escape key if isDismissable is false', function () { | ||
let onClose = jest.fn(); | ||
let res = render(<Example isOpen onClose={onClose} isDismissable={false} />); | ||
let res = render( | ||
<Example isOpen onClose={onClose} isDismissable={false} /> | ||
); | ||
let el = res.getByTestId('test'); | ||
fireEvent.keyDown(el, {key: 'Escape'}); | ||
expect(onClose).toHaveBeenCalledTimes(1); | ||
|
@@ -127,10 +147,90 @@ describe('useOverlay', function () { | |
installPointerEvent(); | ||
it('should prevent default on pointer down on the underlay', function () { | ||
let underlayRef = React.createRef(); | ||
render(<Example isOpen isDismissable underlayProps={{ref: underlayRef}} />); | ||
let isPrevented = fireEvent.pointerDown(underlayRef.current, {button: 0, pointerId: 1}); | ||
render( | ||
<Example isOpen isDismissable underlayProps={{ref: underlayRef}} /> | ||
); | ||
let isPrevented = fireEvent.pointerDown(underlayRef.current, { | ||
button: 0, | ||
pointerId: 1 | ||
}); | ||
fireEvent.pointerUp(document.body); | ||
expect(isPrevented).toBeFalsy(); // meaning the event had preventDefault called | ||
}); | ||
}); | ||
}); | ||
|
||
describe('useOverlay with shadow dom', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think these tests are going to be enough, usually overlays portal out to be a direct child of the This can also be changed with UNSTABLE_PortalProvider We'll need tests using the RAC Popover at a minimum |
||
beforeAll(() => { | ||
enableShadowDOM(); | ||
}); | ||
|
||
describe.each` | ||
type | prepare | actions | ||
${'Mouse Events'} | ${installMouseEvent} | ${[(el) => fireEvent.mouseDown(el, {button: 0}), (el) => fireEvent.mouseUp(el, {button: 0})]} | ||
${'Pointer Events'} | ${installPointerEvent} | ${[(el) => fireEvent.pointerDown(el, {button: 0, pointerId: 1}), (el) => fireEvent.pointerUp(el, {button: 0, pointerId: 1})]} | ||
${'Touch Events'} | ${() => {}} | ${[(el) => fireEvent.touchStart(el, {changedTouches: [{identifier: 1}]}), (el) => fireEvent.touchEnd(el, {changedTouches: [{identifier: 1}]})]} | ||
`('$type', ({actions: [pressStart, pressEnd], prepare}) => { | ||
prepare(); | ||
|
||
it('should not close the overlay when clicking outside if shouldCloseOnInteractOutside returns true', function () { | ||
const {shadowRoot, shadowHost} = createShadowRoot(); | ||
|
||
let onClose = jest.fn(); | ||
let underlay; | ||
|
||
const WrapperComponent = () => | ||
ReactDOM.createPortal( | ||
<Example | ||
isOpen | ||
onClose={onClose} | ||
isDismissable | ||
shouldCloseOnInteractOutside={(target) => { | ||
return target === underlay; | ||
}} />, | ||
shadowRoot | ||
); | ||
|
||
const {unmount} = render(<WrapperComponent />); | ||
|
||
underlay = shadowRoot.querySelector("[data-testid='underlay']"); | ||
|
||
pressStart(underlay); | ||
pressEnd(underlay); | ||
expect(onClose).toHaveBeenCalled(); | ||
|
||
// Cleanup | ||
unmount(); | ||
document.body.removeChild(shadowHost); | ||
}); | ||
|
||
it('should not close the overlay when clicking outside if shouldCloseOnInteractOutside returns false', function () { | ||
const {shadowRoot, shadowHost} = createShadowRoot(); | ||
|
||
let onClose = jest.fn(); | ||
let underlay; | ||
|
||
const WrapperComponent = () => | ||
ReactDOM.createPortal( | ||
<Example | ||
isOpen | ||
onClose={onClose} | ||
isDismissable | ||
shouldCloseOnInteractOutside={(target) => target !== underlay} />, | ||
shadowRoot | ||
); | ||
|
||
const {unmount} = render(<WrapperComponent />); | ||
|
||
underlay = shadowRoot.querySelector("[data-testid='underlay']"); | ||
|
||
pressStart(underlay); | ||
pressEnd(underlay); | ||
expect(onClose).not.toHaveBeenCalled(); | ||
|
||
// Cleanup | ||
unmount(); | ||
document.body.removeChild(shadowHost); | ||
}); | ||
}); | ||
}); |
Uh oh!
There was an error while loading. Please reload this page.