onPress Issue - Click Through Component When it Unmounts

[mmarcuccio]

Provide a general summary of the issue here

In a certain case, onPress is sometimes firing onClick event handlers for elements beneath the target. This occurs when the onPress handler updates state which results in the component being unmounted.

🤔 Expected Behavior?

When using a touch device to press a react-aria-components Button (which usesusePress internally), it is expected that the Button will prevent propagation of that event.

This is usually the case as you can see below. In this case, we have two buttons. The top button is a react-aria-components Button using onPress. The element beneath is a standard HTML button with an onClick handler that fires an alert if clicked.

😯 Current Behavior

However, if the onPress handler results in the component being unmounted, the event sometimes propagates to the element beneath (seems to be inconsistent - maybe a race condition?).

Below is the same situation as before, but we conditionally render the react-aria-components Button based on React state, and in the onPress handler we toggle that state.

https://codesandbox.io/p/sandbox/suspicious-wilbur-k6v4l6

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

Code sandbox repro link: https://codesandbox.io/p/sandbox/suspicious-wilbur-k6v4l6

1. Open Chrome dev tools and simulate a mobile device via device mode
2. Tap the top button.
3. onPress Event handler fires for top button, and top button is removed due to state update.
4. onClick Event handler fires for button underneath.
   This might take a few tries as it is inconsistent.

Version

Latest react-aria-components 1.7.1

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Mac OS 15.3.1

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[snowystinger]

This is a duplicate of When selecting a Menu item, element behind popup receives pointer event

In addition, I cannot reproduce it in my Chrome, the browser fix for this was very recently rolled out. What version of Chrome are you on? I am on 134.0.6998.89
https://issues.chromium.org/issues/40851596

[mmarcuccio]

This is a duplicate of When selecting a Menu item, element behind popup receives pointer event

In addition, I cannot reproduce it in my Chrome, the browser fix for this was very recently rolled out. What version of Chrome are you on? I am on 134.0.6998.89 https://issues.chromium.org/issues/40851596

Version 134.0.6998.166

[mmarcuccio]

@snowystinger it looks like the chrome fix was merged Mar 10. I can also repro the same issue in Chrome Canary (nightly) which should include this change.

Is it possible there's another underlying issue unrelated to the Chromium bug?

[snowystinger]

For some reason I can reproduce it if I pull down the codesandbox and build it locally or if I show the codesandbox preview in a new window instead of the iframe. So the iframe is messing with it somehow.

That aside, I see we are indeed calling stopPropagation.
https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/interactions/src/usePress.ts#L375

Which should work, according to this test:
https://codesandbox.io/p/sandbox/onpress-test-forked-rf3dk2?file=%2Fsrc%2FApp.tsx%3A52%2C21

I also tried adding to our onClick handler in usePress

e.nativeEvent.stopPropagation()

to no effect.

I am not sure what is going on right now, we'll need to dive deeper into it.

[mmarcuccio]

Thinking about this further, these elements are not nested. It's unlikely to be an issue tied to event propagation since there would be no event bubbling shared between them.

What if this is just an issue with timing? Perhaps the onPress handler executes so much earlier than than the click event that by the time the click event fires the button on top is gone (so the click happens on the button underneath).

[snowystinger]

Ah, good point, I glossed over the nesting/propagation. onPress was recently (in the latest release that we're using) to fire in the onClick handler. As can be seen here: https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/interactions/src/usePress.ts#L368

[mmarcuccio]

This behavior can also be experienced in more common situations such as Modals. For example, when closing a modal in onPress, the user can end up triggering an onClick on a component on the content behind it (this issue is what prompted me to create the minimal repro).

It can also happen when content is not initially overlapping. For example, a route change. The user can click on a react aria Button that changes the route and end up triggering onClick for a component on the new route. Here's an example: https://codesandbox.io/p/sandbox/modern-cherry-8vlq8p

Reading through the usePress refactor PR, one of the notes is:

We also now delay firing onPress until the onClick event instead of firing it during onPointerUp. This fixes many issues:

• On Android, if you remove the original target between onPointerUp and onClick, the element behind it will be clicked instead.

The description and linked issues seems to describe this issue pretty well. Perhaps this issue was not fully resolved by delaying the firing of onPress.

(Also, updated issue title to be more accurate.)

[filipw01]

I have a similar issue where clicking option in select using useOption triggers a click mouseup event on body although I checked chrome, safari and firefox and the weird body click mouseup is triggered on chrome and firefox, but not on safari. I'll try to create a reproduction

[filipw01]

I think it might be a separate issue, because I had shouldSelectOnPressUp: true and changing to false resolved the issue

[filipw01]

I traced it down to usePress stopping propagation for mouseDown, but not mouseUp which broke it for me

[snowystinger]

Thanks for sharing your reproduction and findings as well. I'm a little confused as to how shouldSelectOnPressUp: false would work, that would theoretically remove the element that was being clicked even earlier?

I think this has an issue if the Select is inside a Modal, see GridList (in Popover) closes containing Modal on select

[mmarcuccio]

@snowystinger I was wondering if it's possible to share any prioritization or timeline details around this issue. The issue is fairly high impact, and that information would help us to plan how to mitigate things in the meantime.

[snowystinger]

Right now, there isn't prioritization on this bug. The fastest way to get prioritization is to submit a PR addressing it.

Another way to get priority is to determine the root cause. So answering if the original issue was fully resolved:

The description and linked issues seems to describe this issue pretty well. Perhaps this issue was not fully resolved by delaying the firing of onPress.

Why would delaying to fire onPress during onClick not resolve this? It seems like it should have. So what about that isn't working?

If we know the answers to this, then we can more easily come up with possible solutions.

Otherwise, I have to ask for patience.

[mmarcuccio]

Understood, and thank you for the transparency and the detailed explanation. I'll see if we can get any spare cycles to help dig into the root cause.

[devongovett]

Looks like due to the click event being delayed by android, and therefore this timer gets hit:

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

Lines 544 to 553 in cfe79d3

	let timeout = setTimeout(() => {
	if (state.isPressed && state.target instanceof HTMLElement) {
	if (clicked) {
	cancel(e);
	} else {
	focusWithoutScrolling(state.target);
	state.target.click();
	}
	}
	}, 80);

If you add touch-action: manipulation to your styles, it should disable the onClick delay. https://developer.chrome.com/blog/300ms-tap-delay-gone-away Not sure if we can do this by default

[mmarcuccio]

@devongovett we initially had touch-action: manipulation set. However, this actually breaks onPress in iOS Safari when an ancestor element is horizontally scrollable. When you touch and drag to scroll the contents within parent container, onPress fires when it should not. Here's a repro: https://codesandbox.io/p/sandbox/onpresstouchaction-whxrjx

[devongovett]

Oh that's strange. It should be firing onPointerCancel when you scroll, but apparently Safari doesn't do that? 🤦

Edit: looks like https://bugs.webkit.org/show_bug.cgi?id=240917

According to that, setting touch-action: pan-x pan-y pinch-zoom should work?

[mmarcuccio]

Oh that's strange. It should be firing onPointerCancel when you scroll, but apparently Safari doesn't do that? 🤦

Edit: looks like https://bugs.webkit.org/show_bug.cgi?id=240917

According to that, setting touch-action: pan-x pan-y pinch-zoom should work?

That does indeed fix the issues posted above in both iOS Safari and Chrome on Android 🎉

Because we are using usePress in a set of reusable components this should fully resolve the issue for my use case.

What would be the best approach to handling this for usePress more globally?