Skip to content

Commit 67f932f

Browse files
benoitgrelardbenoitgrelard
authored
[DismissableLayer] Fix pointer-events issues (#1401)
* [DismissableLayer] Fix pointer-events issues Fixes #1241 Fixes #1263 * PR feedback * Add cypress tests * testing flake * Fix flake? * Fix false positive * Copy change
1 parent cb6d230 commit 67f932f

File tree

11 files changed

+321
-136
lines changed

11 files changed

+321
-136
lines changed

‎.yarn/versions/a70a7dfc.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
releases:
2+
"@radix-ui/react-alert-dialog": patch
3+
"@radix-ui/react-context-menu": patch
4+
"@radix-ui/react-dialog": patch
5+
"@radix-ui/react-dismissable-layer": patch
6+
"@radix-ui/react-dropdown-menu": patch
7+
"@radix-ui/react-hover-card": patch
8+
"@radix-ui/react-menu": patch
9+
"@radix-ui/react-navigation-menu": patch
10+
"@radix-ui/react-popover": patch
11+
"@radix-ui/react-select": patch
12+
"@radix-ui/react-toast": patch
13+
"@radix-ui/react-tooltip": patch
14+
15+
declined:
16+
- primitives

‎cypress/integration/Dialog.spec.ts‎

Lines changed: 192 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,198 @@
11
describe('Dialog', () => {
22
beforeEach(() => {
3-
cy.visitStory('dialog--styled');
3+
cy.visitStory('dialog--cypress');
44
});
55

6-
it('should open and close correctly', () => {
7-
cy.findByText('open').click();
8-
cy.findByText('close').click();
9-
cy.findByText('close').should('not.exist');
6+
function shouldBeOpen() {
7+
cy.findByText('title').should('exist');
8+
}
9+
function shouldBeClosed() {
10+
cy.findByText('title').should('not.exist');
11+
}
12+
function shouldNotAllowOutsideInteraction(action: 'realClick' | 'realTouch') {
13+
cy.findByLabelText('count up')
14+
.invoke('text')
15+
.then((count) => {
16+
if (action === 'realTouch') {
17+
cy.findByLabelText('count up').realClick();
18+
} else {
19+
cy.findByLabelText('count up').realTouch();
20+
}
21+
cy.findByLabelText('count up').should('have.text', count);
22+
});
23+
}
24+
function shouldAllowOutsideInteraction(action: 'realClick' | 'realTouch') {
25+
cy.findByLabelText('count up')
26+
.invoke('text')
27+
.then((count) => {
28+
if (action === 'realTouch') {
29+
cy.findByLabelText('count up').realClick();
30+
} else {
31+
cy.findByLabelText('count up').realTouch();
32+
}
33+
cy.findByLabelText('count up').should('not.have.text', count);
34+
});
35+
}
36+
37+
describe('given a modal dialog', () => {
38+
it('can be open/closed with a keyboard', () => {
39+
// using keyboard on open/close buttons
40+
cy.findByText('open').focus();
41+
cy.realPress('Space');
42+
shouldBeOpen();
43+
cy.findByText('close').should('be.focused');
44+
cy.realPress('Space');
45+
shouldBeClosed();
46+
cy.findByText('open').should('be.focused');
47+
48+
// using keyboard on open button and close with escape
49+
cy.realPress('Space');
50+
shouldBeOpen();
51+
cy.realPress('Escape');
52+
shouldBeClosed();
53+
});
54+
55+
it('can be open/closed with a pointer', () => {
56+
// using pointer on open/close buttons
57+
cy.findByText('open').click();
58+
shouldBeOpen();
59+
cy.findByText('close').should('be.focused').click();
60+
shouldBeClosed();
61+
cy.findByText('open').should('be.focused');
62+
63+
// using mouse inside dialog, then on a button outside
64+
cy.findByText('open').click();
65+
shouldBeOpen();
66+
cy.findByText('title').click();
67+
shouldBeOpen();
68+
shouldNotAllowOutsideInteraction('realClick');
69+
shouldBeClosed();
70+
71+
// using touch on a button outside
72+
cy.findByText('open').click();
73+
shouldBeOpen();
74+
shouldNotAllowOutsideInteraction('realTouch');
75+
shouldBeClosed();
76+
77+
// using mouse on an input outside
78+
cy.findByText('open').click();
79+
shouldBeOpen();
80+
cy.findByPlaceholderText('name').realClick();
81+
cy.findByPlaceholderText('name').should('not.be.focused');
82+
shouldBeClosed();
83+
84+
// using touch on an input outside
85+
cy.findByText('open').click();
86+
shouldBeOpen();
87+
cy.findByPlaceholderText('name').realTouch();
88+
cy.findByPlaceholderText('name').should('not.be.focused');
89+
shouldBeClosed();
90+
91+
// turn on animation
92+
cy.findByLabelText('animated').click();
93+
94+
// using mouse on an input outside an animated dialog
95+
cy.findByText('open').click();
96+
shouldBeOpen();
97+
cy.findByPlaceholderText('name').realClick();
98+
cy.findByPlaceholderText('name').should('not.be.focused');
99+
shouldBeClosed();
100+
101+
// finally, ensure that pointer-events have been reset and interactions restored
102+
shouldAllowOutsideInteraction('realClick');
103+
104+
// using touch on an input outside an animated dialog
105+
cy.findByText('open').click();
106+
shouldBeOpen();
107+
cy.findByPlaceholderText('name').realTouch();
108+
cy.findByPlaceholderText('name').should('not.be.focused');
109+
shouldBeClosed();
110+
111+
// finally, ensure that pointer-events have been reset and interactions restored
112+
shouldAllowOutsideInteraction('realTouch');
113+
});
114+
});
115+
116+
describe('given a non-modal dialog', () => {
117+
beforeEach(() => {
118+
cy.findByLabelText('modal').click();
119+
});
120+
121+
it('can be open/closed with a keyboard', () => {
122+
// using keyboard on open/close buttons
123+
cy.findByText('open').focus();
124+
cy.realPress('Space');
125+
shouldBeOpen();
126+
cy.findByText('close').should('be.focused');
127+
cy.realPress('Space');
128+
shouldBeClosed();
129+
cy.findByText('open').should('be.focused');
130+
131+
// using keyboard on open button and close with escape
132+
cy.realPress('Space');
133+
shouldBeOpen();
134+
cy.realPress('Escape');
135+
shouldBeClosed();
136+
});
137+
138+
it('can be open/closed with a pointer', () => {
139+
// using pointer on open/close buttons
140+
cy.findByText('open').click();
141+
shouldBeOpen();
142+
cy.findByText('close').should('be.focused').click();
143+
shouldBeClosed();
144+
cy.findByText('open').should('be.focused');
145+
146+
// using mouse inside dialog, then on a button outside
147+
cy.findByText('open').click();
148+
shouldBeOpen();
149+
cy.findByText('title').click();
150+
shouldBeOpen();
151+
shouldAllowOutsideInteraction('realClick');
152+
shouldBeClosed();
153+
154+
// using touch on a button outside
155+
cy.findByText('open').click();
156+
shouldBeOpen();
157+
shouldAllowOutsideInteraction('realTouch');
158+
shouldBeClosed();
159+
160+
// using mouse on an input outside
161+
cy.findByText('open').click();
162+
shouldBeOpen();
163+
cy.findByPlaceholderText('name').realClick();
164+
cy.findByPlaceholderText('name').should('be.focused');
165+
shouldBeClosed();
166+
167+
// using touch on an input outside
168+
cy.findByText('open').click();
169+
shouldBeOpen();
170+
cy.findByPlaceholderText('name').realTouch();
171+
cy.findByPlaceholderText('name').should('be.focused');
172+
shouldBeClosed();
173+
174+
// turn on animation
175+
cy.findByLabelText('animated').click();
176+
177+
// using mouse on an input outside an animated dialog
178+
cy.findByText('open').click();
179+
shouldBeOpen();
180+
cy.findByPlaceholderText('name').realClick();
181+
cy.findByPlaceholderText('name').should('be.focused');
182+
shouldBeClosed();
183+
184+
// finally, ensure that pointer-events have been reset and interactions restored
185+
shouldAllowOutsideInteraction('realClick');
186+
187+
// using touch on an input outside an animated dialog
188+
cy.findByText('open').click();
189+
shouldBeOpen();
190+
cy.findByPlaceholderText('name').realTouch();
191+
cy.findByPlaceholderText('name').should('be.focused');
192+
shouldBeClosed();
193+
194+
// finally, ensure that pointer-events have been reset and interactions restored
195+
shouldAllowOutsideInteraction('realTouch');
196+
});
10197
});
11198
});

‎packages/react/dialog/src/Dialog.stories.tsx‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,71 @@ export const Chromatic = () => (
470470
);
471471
Chromatic.parameters = { chromatic: { disable: false } };
472472

473+
export const Cypress = () => {
474+
const [modal, setModal] = React.useState(true);
475+
const [animated, setAnimated] = React.useState(false);
476+
const [count, setCount] = React.useState(0);
477+
478+
return (
479+
<>
480+
<Dialog.Root modal={modal}>
481+
<Dialog.Trigger className={triggerClass()}>open</Dialog.Trigger>
482+
<Dialog.Portal>
483+
<Dialog.Content
484+
className={
485+
animated
486+
? animatedContentClass({ css: { animationDuration: '50ms !important' } })
487+
: contentDefaultClass()
488+
}
489+
>
490+
<Dialog.Title>title</Dialog.Title>
491+
<Dialog.Description>description</Dialog.Description>
492+
<Dialog.Close className={closeClass()}>close</Dialog.Close>
493+
</Dialog.Content>
494+
</Dialog.Portal>
495+
</Dialog.Root>
496+
497+
<br />
498+
<br />
499+
500+
<label>
501+
<input
502+
type="checkbox"
503+
checked={modal}
504+
onChange={(event) => setModal(Boolean(event.target.checked))}
505+
/>{' '}
506+
modal
507+
</label>
508+
509+
<br />
510+
511+
<label>
512+
<input
513+
type="checkbox"
514+
checked={animated}
515+
onChange={(event) => setAnimated(Boolean(event.target.checked))}
516+
/>{' '}
517+
animated
518+
</label>
519+
520+
<br />
521+
522+
<label>
523+
count up{' '}
524+
<button type="button" onClick={() => setCount((count) => count + 1)}>
525+
{count}
526+
</button>
527+
</label>
528+
529+
<br />
530+
531+
<label>
532+
name: <input type="text" placeholder="name" />
533+
</label>
534+
</>
535+
);
536+
};
537+
473538
const triggerClass = css({});
474539

475540
const RECOMMENDED_CSS__DIALOG__OVERLAY: any = {

‎packages/react/dismissable-layer/package.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
"@radix-ui/primitive": "workspace:*",
2121
"@radix-ui/react-compose-refs": "workspace:*",
2222
"@radix-ui/react-primitive": "workspace:*",
23-
"@radix-ui/react-use-body-pointer-events": "workspace:*",
2423
"@radix-ui/react-use-callback-ref": "workspace:*",
2524
"@radix-ui/react-use-escape-keydown": "workspace:*"
2625
},

‎packages/react/dismissable-layer/src/DismissableLayer.tsx‎

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as React from 'react';
22
import { composeEventHandlers } from '@radix-ui/primitive';
33
import { Primitive, dispatchDiscreteCustomEvent } from '@radix-ui/react-primitive';
44
import { useComposedRefs } from '@radix-ui/react-compose-refs';
5-
import { useBodyPointerEvents } from '@radix-ui/react-use-body-pointer-events';
65
import { useCallbackRef } from '@radix-ui/react-use-callback-ref';
76
import { useEscapeKeydown } from '@radix-ui/react-use-escape-keydown';
87

@@ -17,6 +16,8 @@ const CONTEXT_UPDATE = 'dismissableLayer.update';
1716
const POINTER_DOWN_OUTSIDE = 'dismissableLayer.pointerDownOutside';
1817
const FOCUS_OUTSIDE = 'dismissableLayer.focusOutside';
1918

19+
let originalBodyPointerEvents: string;
20+
2021
const DismissableLayerContext = React.createContext({
2122
layers: new Set<DismissableLayerElement>(),
2223
layersWithOutsidePointerEventsDisabled: new Set<DismissableLayerElement>(),
@@ -106,13 +107,25 @@ const DismissableLayer = React.forwardRef<DismissableLayerElement, DismissableLa
106107
if (!event.defaultPrevented) onDismiss?.();
107108
});
108109

109-
useBodyPointerEvents({ disabled: disableOutsidePointerEvents });
110-
111110
React.useEffect(() => {
112111
if (!node) return;
113-
if (disableOutsidePointerEvents) context.layersWithOutsidePointerEventsDisabled.add(node);
112+
if (disableOutsidePointerEvents) {
113+
if (context.layersWithOutsidePointerEventsDisabled.size === 0) {
114+
originalBodyPointerEvents = document.body.style.pointerEvents;
115+
document.body.style.pointerEvents = 'none';
116+
}
117+
context.layersWithOutsidePointerEventsDisabled.add(node);
118+
}
114119
context.layers.add(node);
115120
dispatchUpdate();
121+
return () => {
122+
if (
123+
disableOutsidePointerEvents &&
124+
context.layersWithOutsidePointerEventsDisabled.size === 1
125+
) {
126+
document.body.style.pointerEvents = originalBodyPointerEvents;
127+
}
128+
};
116129
}, [node, disableOutsidePointerEvents, context]);
117130

118131
/**
@@ -206,14 +219,41 @@ type FocusOutsideEvent = CustomEvent<{ originalEvent: FocusEvent }>;
206219
function usePointerDownOutside(onPointerDownOutside?: (event: PointerDownOutsideEvent) => void) {
207220
const handlePointerDownOutside = useCallbackRef(onPointerDownOutside) as EventListener;
208221
const isPointerInsideReactTreeRef = React.useRef(false);
222+
const handleClickRef = React.useRef(() => {});
209223

210224
React.useEffect(() => {
211225
const handlePointerDown = (event: PointerEvent) => {
212226
if (event.target && !isPointerInsideReactTreeRef.current) {
213227
const eventDetail = { originalEvent: event };
214-
handleAndDispatchCustomEvent(POINTER_DOWN_OUTSIDE, handlePointerDownOutside, eventDetail, {
215-
discrete: true,
216-
});
228+
229+
function handleAndDispatchPointerDownOutsideEvent() {
230+
handleAndDispatchCustomEvent(
231+
POINTER_DOWN_OUTSIDE,
232+
handlePointerDownOutside,
233+
eventDetail,
234+
{ discrete: true }
235+
);
236+
}
237+
238+
/**
239+
* On touch devices, we need to wait for a click event because browsers implement
240+
* a ~350ms delay between the time the user stops touching the display and when the
241+
* browser executres events. We need to ensure we don't reactivate pointer-events within
242+
* this timeframe otherwise the browser may execute events that should have been prevented.
243+
*
244+
* Additionally, this also lets us deal automatically with cancellations when a click event
245+
* isn't raised because the page was considered scrolled/drag-scrolled, long-pressed, etc.
246+
*
247+
* This is why we also continuously remove the previous listener, because we cannot be
248+
* certain that it was raised, and therefore cleaned-up.
249+
*/
250+
if (event.pointerType === 'touch') {
251+
document.removeEventListener('click', handleClickRef.current);
252+
handleClickRef.current = handleAndDispatchPointerDownOutsideEvent;
253+
document.addEventListener('click', handleClickRef.current, { once: true });
254+
} else {
255+
handleAndDispatchPointerDownOutsideEvent();
256+
}
217257
}
218258
isPointerInsideReactTreeRef.current = false;
219259
};
@@ -236,6 +276,7 @@ function usePointerDownOutside(onPointerDownOutside?: (event: PointerDownOutside
236276
return () => {
237277
window.clearTimeout(timerId);
238278
document.removeEventListener('pointerdown', handlePointerDown);
279+
document.removeEventListener('click', handleClickRef.current);
239280
};
240281
}, [handlePointerDownOutside]);
241282

0 commit comments

Comments
 (0)