When selecting a Menu item, element behind popup receives pointer event

[stefanprobst]

🐛 Bug Report

When a Menu item is selected on a touch device, interactive elements behind the menu popup receive the pointer event as well.

🤔 Expected Behavior

Interactive elements behind a menu popup should do nothing when selecting a menu item.

😯 Current Behavior

See this codesandbox for an example with buttons behind a menu popup. When selecting a menu item, the button also gets clicked (and displays the alert).

menu.mp4

Note that this only happens on mobile devices, and when the pointer device simulation is turned on in browser devtools.

💁 Possible Solution

🔦 Context

💻 Code Sample

https://codesandbox.io/s/react-spectrum-menu-pointer-events-wze00?file=/src/App.js

🌍 Your Environment

Software	Version(s)
react-spectrum	3.7.0
Browser	Firefox, Chrome
Operating System

[snowystinger]

Double checked, doesn't happen in Safari. Also still happens with using our Buttons
https://codesandbox.io/s/react-spectrum-menu-pointer-events-forked-8u83p?file=/src/App.js
The next step would be to reproduce this with just React or even better, just javascript, so we can figure out what's actually going on. I have a hunch it's related to https://bugs.chromium.org/p/chromium/issues/detail?id=1150073 but we should make sure because our Buttons don't rely on 'click' in most cases

[stefanprobst]

• interestingly, with rsp buttons (your codesandbox example) i can not reproduce in firefox anymore, but still in chrome
• however, when putting links with useLink behind the popup, i can still reproduce with both firefox and chrome (see here)

[snowystinger]

Looked into it more, it does appear that this is the chromium bug I referenced earlier.

You are right about FF, honestly don't know what I was doing there, I forgot to turn on touch simulation. The reason that it's not causing an issue with our buttons is that we actually have a line in our code that will ignore the click if it's simulated https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/interactions/src/utils.ts#L24, but it looks like Firefox is actually plagued by the same bug. I've opened a ticket with them here: https://bugzilla.mozilla.org/show_bug.cgi?id=1692033

As for useLink, I believe that to be that links have their own click event default handler, which we do not prevent https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/interactions/src/usePress.ts#L226 and is still the same bugs as above.

For reference, here's the minimal reproduction I could think of https://codesandbox.io/s/inspiring-lehmann-i1hjf
And here's the same think with links https://codesandbox.io/s/trusting-poitras-u89ub?file=/index.html which is a more compelling case I think, so I've updated both bugs with a comment about it in hopes that gets it more attention.

[snowystinger]

@manelsanz Yes, the bug notes that it happens in FF as well. I would disagree that it's a library issue. You can see the issue happen without our library. I do understand what you're saying though, that this should be something we can help mitigate or work around. Do you have any ideas for solving it?

@equinusocio I'm not really sure what you're saying. Are you suggesting a way of fixing it? Are you using pointer events? or what do you use? We've found that there are issues if you try to mix pointer events and mouse events. So we're stuck with using pointer events for this.

Some ideas I'd had but haven't had a chance to implement.

• delay closing of the overlay by a tick, might be unexpected by tests so potentially a lot of things would need updating not just in our library, but in users tests as well
• apply user events none to everything that is currently aria-hidden (seems costly)
• global listener for the start of the event chain so we know if it's actually a virtual event or something that leaked through (I'm fairly sure we treat a click event alone as virtual) but this doesn't help when the thing behind that's getting the click event isn't from our library

[snowystinger]

@equinusocio
I'm not sure react-focus-on solves it, see https://codesandbox.io/s/react-focus-on-react-spring-forked-rfzt4?file=/src/index.js

[equinusocio]

@snowystinger Dunno why by using react-focus-on we don't have that issue. Anyway as you can see from the sandbox, the issue is not related to the responsive mode, since it occurs even with the normal view.

[hernanponcetta]

Possible workaround here.

Rember to:

1. Enter dev tools.
2. Enable touch emulation.

For the record, this is how browsers should behave when users interact using a touch device.

Quoting W3C:

To avoid processing the same interaction twice for touch (once for the touch event, and once for the compatibility mouse events), developers should make sure to cancel the touch event, suppressing the generation of any further mouse or click events. Alternatively, see the InputDeviceCapabilities API for a way to detect mouse events that were generated as a result of touch events

I think that in this case, React is failing to preventDefault touch events. One possible workaround is to use the Web API to add an event listener to the interactive element, capture the touch event and prevent the default behavior. This way the compatibility mouse events won´t happen.

Here an example of React failing to prevent compatibility events.

[snowystinger]

@hernanponcetta thanks for that extra info. Looks like React is both aware of it and not going to do anything about it facebook/react#9809

Interesting, it does appear that you must cancel the touchstart, doing it on pointerdown isn't enough.

The issue logged against the browsers has more to do with click being fired on the wrong element because the original target has been removed. So there are two things happening here. I think cancelling the touchstart though is a good direction to try first, it'd render the other issue a moot point if it works for us. I'm not sure how we'd put it on everything though since not everything may use usePress. But maybe it's enough for us to handle it there and in useInteractOutside. That should cover a lot of it.

[chrisspadanuta]

Have there been an updates on fixing this? I see there's a PR that is WIP that hasn't been updated since early April. For the record, i see this happening on real phone and tablet devices, not just Chrome's dev tools.

[snowystinger]

Hey, we've been trying things, as you can see in the history of the ticket. We've been unable to find a satisfactory workaround to this browser bug thus far and other priorities took us away from looking at it. It could help to chime in on the tickets that were opened against FF and Chrome. We're aware that this happens on real devices as well, unfortunately. We welcome any fresh ideas on how to work around it while we wait for browsers to fix this issue.

[hernanponcetta]

@chrisspadanuta here is an example of the workaround we are using.

// Workaround for react/react-aria #1513
useEffect(() => {
  ref.current?.addEventListener('touchstart', (event: TouchEvent) => {
    event.preventDefault();
  });
}, []);

Hope that it helps.

[peteragarclover]

Thanks @hernanponcetta, that workaround is working for us too.

[Slooowpoke]

@hernanponcetta's version works fine but breaks scrolling on the elements.

Using this keeps the scroll, but has a performance impact on scrolling (from what I understand, though seemed negligible on faster devices)

    useEffect(() => {
        ref.current?.addEventListener('touchend', (e) => {
                e.preventDefault();
        }, { passive: false, once: true });
    }, []);

[LFDanLu]

Another workaround comes from #4027 where you can control the open state of your menu and call close after a slight delay: https://codesandbox.io/s/react-spectrum-menu-pointer-events-forked-w9wxog?file=/src/App.js

[tgelu]

@hernanponcetta's version works fine but breaks scrolling on the elements.

Using this keeps the scroll, but has a performance impact on scrolling (from what I understand, though seemed negligible on faster devices)

    useEffect(() => {
        ref.current?.addEventListener('touchend', (e) => {
                e.preventDefault();
        }, { passive: false, once: true });
    }, []);

Just want to highlight two edge cases where this breaks normal behavior:

• using it on buttons with type="submit" will block form submission on touch devices.
• using this on links will prevent opening the link on touch devices (this can happen if you have buttons that are link underneath)

I use something like this:

  useEffect(() => {
    ref.current?.addEventListener(
      'touchend',
      (e) => {
        /**
         * The first condition is necessary to avoid the issue that a button with
         * type="submit" inside a <form> would not trigger the form submit
         * event on a device with touch input.
         *
         * The second condition is needed for thse buttons that are links
         * underneath and will have a href. If "touchend" is prevented, links
         * won't open by touch.
         *
         * Admittedly, this is an ugly patch, but given the react-spectrum
         * issue it seems necessary. If this hook grows any larger after
         * further bug reports and edge cases, we may want to reconsider
         * this approach.
         */
        if (
          (e.currentTarget as HTMLElement)?.getAttribute('type') !== 'submit' &&
          typeof (e.currentTarget as HTMLElement)?.getAttribute('href') !== 'string'
        ) {
          e.preventDefault();
        }
      },
      { passive: false }
    );
  }, []);

[dnaploszek]

Anyone had any issues with android browsers with this workaround? We got multiple reports from users on some android devices that our buttons stoped working... We can quite consistently reproduce the issue on browserstack, but cannot find a suitable fix for it.

Been pulling my hair on it and considered deferring the execution of button click with setTimeout or requestAnimationFrame, but this completely breaks our jest tests (all clicks need runAllTimers to work).

Was there any development to find a solution for this issue? It's been open for a long time and it surely is a problem for a lot of users or react-aria.

[mryechkin]

Was there any development to find a solution for this issue? It's been open for a long time and it surely is a problem for a lot of users or react-aria.

Ran into this issue with a Dialog and a Select (using hooks) and am curious as well.

[dnaploszek]

I would like to also bring this issue to attention as all usePress interactions are affected by this problem which can be proved by the Demo present in a similar issue.

[Inkvii]

Tried both workarounds - the one with useEffect doesn't work for me at all (for some reason the touchend/touchstart event is never triggered), second workaround using setTimeout in onChange event handler seem to fix it only for chrome touch simulator mode. Using real devices (tested on ipad and iphone) in safari causes the pointer events on components beneath it.

Further testing with different libraries shows that this must be problem in the library itself because for example radixui can handle popovers just fine. Have you considered using similar approach as them?

EDIT: I looked more closely at their select component example and they have similar issue. There are mentioned few approaches that could potentially solve it.

1. transparent overlay right beneath the popover - might have to do some z-index calculation
2. add touchend listener directly on popover reference

<Popover ref={(ref) => ref?.addEventListener('touchend', (e) => e.preventDefault())} >
      <ReactAriaListBox className={theme.pickerList} items={props.items} data-testid={"picker-list"}>
        {props.children}
      </ReactAriaListBox>
</Popover>

Im not sure if no. 2 cancels something that shouldn't be cancelled

[sbdchd]

I can reproduce on iOS 17.2.1 Safari -- having trouble with the dev environment, seems like it's only happening in prod 🤔

Edit: after messing around in chrome, replacing the close button in the modal with a <button instead of a <Button from react aria (aka not using usePress) fixes the issue

[jeyj0]

I agree with @snowystinger that my issue #5802 is a dupe of this one.

⚠️ EDIT: right after posting this comment I ran into the issue again while using the workaround 🙈

My current workaround is the below. Feel free to make me aware of any issues with doing it like this—it seems to avoid the issue for my cases:

const { isPressed, buttonProps } = useButton(
    {
      ...useButtonProps,
      onPress(e) {
        if (e.pointerType === "mouse" || e.pointerType === "keyboard") {
          useButtonProps.onPress?.(e);
          return;
        }

        setTimeout(() => {
          useButtonProps.onPress?.(e);
        }, 1);
      },
    },
    ref,
  );

I decided to preemptively only allow mouse and keyboard events to be handled "normally", as I'm not sure what other event types have the issue.

[emamoah]

Ran into this issue also, using <Modal> and <Dialog>. Solved it by calling the modal's onOpenChange handler during the useEffect cycle. Don't know if there are any downsides to that yet (I'd like to know if there are), but just thought I'd share my solution using a <CustomModal> implementation:

import { forwardRef, useEffect, useState } from 'react';
import { Modal, ModalOverlayProps } from 'react-aria-components';

export default forwardRef<HTMLDivElement, ModalOverlayProps>(
  function CustomModal({ onOpenChange, ...props }, ref) {
    const [openSignal, setOpenSignal] = useState<boolean | undefined>();

    useEffect(() => {
      if (openSignal !== undefined) {
        onOpenChange?.(openSignal);
        setOpenSignal(undefined);
      }
    }, [openSignal, onOpenChange]);

    return <Modal ref={ref} onOpenChange={setOpenSignal} {...props} />;
  }
);

[steida]

@devongovett Hi. Why is React Spectrum not using native dialog? The API is weird, but can be fixed:

https://github.com/evoluhq/evolu.me/blob/main/components/Modal.tsx#L83

Btw, I'm considering moving from React Native for Web to React Aria (since I don't need React Native), but mobile issues hold me back. My question is, do you know RNfW internals? It could be a good source of knowledge. Anyway, thank you for your work.

[v-anton]

Hi there! Are there any updates on this?

[jeffijoe]

I have a similar issue where I have a <Menu> which on mobile will be rendered inside a <Modal> - if the list is big enough, a menu item may appear at the same spot as the trigger button, which means the menu item is selected immediately as the trigger is clicked. Here's a video - the first few times I click normally, the last few times I hold down the mouse. Maybe a press should only be recognized if the mouse/touch-down element is in the same tree as the element receiving the mouse/touch-up event?

EDIT: To clarify, this is with a mouse, not touch.

press.mov

[jeffijoe]

@snowystinger @devongovett is there some way to make onPress use basic onClick until this issue is resolved? I understand why onPress exists, but this bug makes for a much worse experience.

It's not just menus, it's any "overlay" type element that disappears when interacted with via onPress. Here's another example:

bug-encoded.mp4

[ritz078]

This is one of the biggest problems we are facing right now. We have a HIT testing layer behind that popovers and this issue breaks so many things.

[devongovett]

I've opened a pr to work around this by preventing default on the "touchend" event inside usePress when it is safe to do so: #7026. But note that it is a workaround to underlying browser issues, so it's not perfect. In particular, if the element inside the overlay that causes it to close is a <button type="submit">, <a href>, <input> etc., then we cannot prevent default without breaking the behavior of the element. We have to assume that the native behavior is desired, and that the press event in these cases won't cause the element to unmount during the "pointerup" event. Also this only fixes things for usePress, any other element using the browser onPointerUp event directly also has this problem.

In addition, it looks like the browsers are making progress on a spec change that would fix this: w3c/pointerevents#474. So hopefully in the future we can remove this workaround.