davidtheclark / react-aria-modal

A fully accessible React modal built according WAI-ARIA Authoring Practices
http://davidtheclark.github.io/react-aria-modal/demo/
MIT License
1.03k stars 96 forks source link

Clicking a portal inside the modal causes `onExit()` to fire. #77

Open ericedem opened 5 years ago

ericedem commented 5 years ago

Use Case

We have a case where we render a contextual menu inside of a modal dialogue. This menu is implemented as a portal (via displaced) and positioned with styling. When clicked, the modal closes.

The Problem

It looks like the problem is that when a click event happens, a raw DOM check is made to see if the clicked DOM node (within the portal) is a child of the modal DOM node.

The trouble is that the portal is a child in the "React" sense, but it is injected into the DOM as a child of the document, not a child of the modal.

Solution

I've been trying to think of a way around this, both as a workaround or and improvement to react-aria-modal. One potential idea I had was to try and keep track of hover states, and keep a this._isHovering flag which would always be true if the hover event is a child, whether it is in a portal or not. Then we check the flag when a click event happens. Not completely sure on this one, but happy to submit a PR if someone can point me in the right direction.

Thanks for the awesome library!

ericedem commented 5 years ago

Ok a quick implementation looks like I'm able to work around the issue by doing something like:

<AriaModal
  onExit={() => {
    if (!this._isHovering) {
      // handle exit
    }
  }
>
  <div
    onMouseEnter={() => { this._isHovering = true }}
    onMouseLeave={() => { this._isHovering = false }}
  >
    { /* portal in here */ }
  </div>
</AriaModal>

It even seems to work on iOS Safari, but I'm not sure that is reliable.

Edit: Also seems to work on iOS Chrome and Fire OS Silk.

davidtheclark commented 5 years ago

Hi @ericedem. As you suggest, tracking the mouse sounds pretty unreliable. I'd be interested in hearing any other ideas for solving this!

ericedem commented 5 years ago

I haven't dug into the React code for portals too deeply, but they are somehow figuring out how to propagate events correctly up the portal chain. I wonder if it would be possible to hook into this logic, instead of looking directly at the dom. Then it would work basically how it does now, just looking at the React component hierarchy instead.

For what anyone else who is hitting this issue: What we have been doing to get around this is to just be careful that we stop propagation on clicks within a nested displaced component. Luckily, there aren't too many places where we need to do this; just a few tooltips, autosuggests, and contextual menus.

ericedem commented 5 years ago

Here is an issue that has since been closed in react discussing this problem, with a workaround: https://github.com/facebook/react/issues/10962

davidtheclark commented 5 years ago

One relatively simple solution that might work is to add an underlayClickExitsExeceptions prop, which takes an array of strings, each of which is a selector; and if the selector matches the parent of a clicked element, the click will not close the modal.

So if you have a Portal popover component inside the modal, you can make it render an element with a distinct selector (e.g. [data-popover]) then provide that selector to underClickExistExceptions, and a click on the popover won't close the modal.

In the absence of a React API that allows for something more automatic, how does that sound?

ericedem commented 5 years ago

I think that sounds great! 👍