Modal calls onClose when it opens

[andrewhl]

Steps

Use a Popup with a Button trigger, that indirectly controls the state of a Modal, which is nested inside a modal wrapper. Pass a callback function to the modal wrapper component as onClose, which resets the state of the parent component. Click the Popup trigger button to open the Modal, and nothing happens. The Modal is calling its onClose event when you click the trigger.

This issue only began to occur when I upgraded to semantic-ui-react v. ^0.78.0.

Expected Result

The Modal onClick event does not fire unless triggered by an onClick event within the context of the Modal, and not its parent context.

Actual Result

The onClose function will be called by the underlying semantic-ui-react Modal when the Popup trigger is clicked. As such, the Modal never opens, because it is 'closed' as it opens.

Version

0.78.0

Testcase

https://codesandbox.io/s/l7p5l52n2m

[welcome]

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

[levithomason]

Thanks!

Temporary Workarounds

In this case, the click on the button propagates up to the modal dimmer. It is triggered as a click on the dimmer and closes the Modal.

1. Rollback to 0.77.2, I'll release a 0.78.1 patch soon.
2. Stop event propagation in the open handler: https://codesandbox.io/s/m52nvln8oy
3. Set closeOnDimmerClick={false} on the modal: https://codesandbox.io/s/ol0kzvvvzq

Problem & Fix

We improved the logic for detecting clicks inside of elements when the result of the click removed the e.target node. In those cases, we need to check if the X/Y position of the click is within the other node. However, we only need to resort to this when there is no e.target.

In the cases that we an e.target and a node, we should always rely on the node.contains() result. Otherwise, we're overzealous in our detection of clicks inside nodes that they may not have been inside of.

PR coming.

[raskam]

Just confirming that upgrade from 0.77.2 to 0.78.2 fixed exactly the same issue in our project. Thanks!