Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ericedem opened this issue Oct 31, 2018 · 6 comments
Open

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

ericedem opened this issue Oct 31, 2018 · 6 comments

Comments

@ericedem
Copy link

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
Copy link
Author

ericedem commented Oct 31, 2018

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
Copy link
Owner

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

@ericedem
Copy link
Author

ericedem commented Dec 4, 2018

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
Copy link
Author

ericedem commented Dec 5, 2018

Here is an issue that has since been closed in react discussing this problem, with a workaround: facebook/react#10962

@davidtheclark
Copy link
Owner

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
Copy link
Author

I think that sounds great! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants