Skip to content

openByClickOn element does not toggle portal open state correctly #140

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

Closed
islemaster opened this issue Mar 23, 2017 · 2 comments
Closed
Milestone

Comments

@islemaster
Copy link

This issue is easily reproduced on react-portal's own demo page.

  1. Click the "Another Portal" button. The portal opens.
  2. Click the "Another Portal" button again.
    Expected: The portal closes, because the click was "outside" the rendered portal.
    Actual: The portal remains open.

(Tested on Google Chrome 56.0.2924.87 (64-bit) Ubuntu 16.04.)

This problem occurs for portals using the openByClickOn and closeOnOutsideClick properties, as the portal on the demo page does.

Though it's not clear from the documentation whether the current behavior is by design, I'd argue that it's not (or shouldn't be) for two reasons:

One, it's a fairly common UI convention for a button that opens a menu to be able to subsequently close it (see GitHub's "add" and "user" menus).

Two, nothing in the current implementation suggests this was intended behavior. In fact, you can set breakpoints and see that clicking the openByClickOn element will enter the handleOutsideMouseClick method, continue into closePortal and actually close the portal for just a moment - but immediately afterward handleWrapperClick fires leading to openPortal so the attempt to close the portal effectively fails.

The root cause seems to be a failure to stop propagation of the click event, as attempted here. It might have something to do with the fact that the 'outside click' being handled is a native event, not a React SyntheticEvent. I'm not sure what the solution is though - I hate to throw preventDefault around but maybe it'd do the trick here?

@tajo
Copy link
Owner

tajo commented Jul 29, 2017

Agree that better UX would be to close. Will look into it.

@tajo tajo added this to the v4 milestone Jul 29, 2017
@tajo
Copy link
Owner

tajo commented Oct 1, 2017

Hey, please check the new major version of react-portal: #157

It's React v16 only since its uses the new official Portal API. There is the first beta released and I would like to get your feedback. I don't have bandwidth to maintain v3 which is very different and full of hacks.

I am happy to discuss this with v4 in mind. If this is still a problem, please re-open/update the issue. Thanks.

@tajo tajo closed this as completed Oct 1, 2017
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