Skip to content

Commit 2aa0612

Browse files
committed
Make sure to cancel press if onClick's propagation is stopped
For #5833
1 parent dac4320 commit 2aa0612

File tree

2 files changed

+91
-4
lines changed

2 files changed

+91
-4
lines changed

packages/@react-aria/interactions/src/usePress.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ export function usePress(props: PressHookProps): PressResult {
256256
let cancel = useEffectEvent((e: EventBase) => {
257257
let state = ref.current;
258258
if (state.isPressed && state.target) {
259-
if (state.isOverTarget && state.pointerType != null) {
259+
if (state.didFirePressStart && state.pointerType != null) {
260260
triggerPressEnd(createEvent(state.target, e), state.pointerType, false);
261261
}
262262
state.isPressed = false;
@@ -499,12 +499,20 @@ export function usePress(props: PressHookProps): PressResult {
499499
// We work around this by triggering a click ourselves after a timeout.
500500
// This timeout is canceled during the click event in case the real one fires first.
501501
// In testing, a 0ms delay is too short. 5ms seems long enough for the browser to fire the real events.
502+
let clicked = false;
502503
let timeout = setTimeout(() => {
503504
if (state.isPressed && state.target instanceof HTMLElement) {
504-
focusWithoutScrolling(state.target);
505-
state.target.click();
505+
if (clicked) {
506+
cancel(e);
507+
} else {
508+
focusWithoutScrolling(state.target);
509+
state.target.click();
510+
}
506511
}
507512
}, 5);
513+
// Use a capturing listener to track if a click occurred.
514+
// If stopPropagation is called it may never reach our handler.
515+
addGlobalListener(e.currentTarget as Document, 'click', () => clicked = true, true);
508516
state.disposables.push(() => clearTimeout(timeout));
509517
} else {
510518
cancel(e);

packages/@react-aria/interactions/test/usePress.test.js

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import {usePress} from '../';
2323
function Example(props) {
2424
let {elementType: ElementType = 'div', style, draggable, ...otherProps} = props;
2525
let {pressProps} = usePress(otherProps);
26-
return <ElementType {...pressProps} style={style} tabIndex="0" draggable={draggable}>{ElementType !== 'input' ? 'test' : undefined}</ElementType>;
26+
return <ElementType {...pressProps} style={style} tabIndex="0" draggable={draggable}>{ElementType !== 'input' ? props.children || 'test' : undefined}</ElementType>;
2727
}
2828

2929
function pointerEvent(type, opts) {
@@ -320,6 +320,85 @@ describe('usePress', function () {
320320
]);
321321
});
322322

323+
it('should cancel press if onClick propagation is stopped', function () {
324+
let events = [];
325+
let addEvent = (e) => events.push(e);
326+
let res = render(
327+
<Example
328+
onPressStart={addEvent}
329+
onPressEnd={addEvent}
330+
onPressChange={pressed => addEvent({type: 'presschange', pressed})}
331+
onPress={addEvent}
332+
onPressUp={addEvent}>
333+
{/* eslint-disable-next-line */}
334+
<div data-testid="inner" onClick={e => e.stopPropagation()} />
335+
</Example>
336+
);
337+
338+
let el = res.getByTestId('inner');
339+
fireEvent(el, pointerEvent('pointerover', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
340+
341+
let shouldFireMouseEvents = fireEvent(el, pointerEvent('pointerdown', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
342+
expect(shouldFireMouseEvents).toBe(true);
343+
344+
let shouldFocus = fireEvent.mouseDown(el);
345+
expect(shouldFocus).toBe(true);
346+
act(() => el.focus());
347+
348+
fireEvent(el, pointerEvent('pointerup', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
349+
fireEvent.mouseUp(el);
350+
351+
let shouldClick = fireEvent.click(el);
352+
expect(shouldClick).toBe(true);
353+
fireEvent(el, pointerEvent('pointerout', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
354+
355+
act(() => jest.advanceTimersByTime(10));
356+
357+
expect(events).toEqual([
358+
{
359+
type: 'pressstart',
360+
target: el.parentElement,
361+
pointerType: 'mouse',
362+
ctrlKey: false,
363+
metaKey: false,
364+
shiftKey: false,
365+
altKey: false,
366+
x: 0,
367+
y: 0
368+
},
369+
{
370+
type: 'presschange',
371+
pressed: true
372+
},
373+
{
374+
type: 'pressup',
375+
target: el.parentElement,
376+
pointerType: 'mouse',
377+
ctrlKey: false,
378+
metaKey: false,
379+
shiftKey: false,
380+
altKey: false,
381+
x: 0,
382+
y: 0
383+
},
384+
{
385+
type: 'pressend',
386+
target: el.parentElement,
387+
pointerType: 'mouse',
388+
ctrlKey: false,
389+
metaKey: false,
390+
shiftKey: false,
391+
altKey: false,
392+
x: 0,
393+
y: 0
394+
},
395+
{
396+
type: 'presschange',
397+
pressed: false
398+
}
399+
]);
400+
});
401+
323402
it('should fire press change events when moving pointer outside target', function () {
324403
let events = [];
325404
let addEvent = (e) => events.push(e);

0 commit comments

Comments
 (0)