onRequestClose closes all modals when using them recursively.

[Ne3l]

Summary:

When using nested react-modals, closing with escape key closes all modals instead of the last one open.

Steps to reproduce:

1. click open modal button
2. inside Modal1, click open modal 2 button
3. When modal 2 is open click escape key for close it.

Expected behavior:

Only modal 2 is closed and modal 1 stays open.

Link to example of issue:

https://codesandbox.io/s/w70xm5y1y7

additional notes

I suppose this is related to this issue, when bubbling events with Modals it gets to the parent component. This behaviour depending on the situation is quite problematic. I think it didn't happened with old version before react 16 CreatePortal but now it does. Gonna try to create another codesandbox to confirm it.

[Ne3l]

Cloned the codesandbox with old react 15 and react-modal 2.X and it works as expected. Same as escape button the rest of events doesn't bubble to the parent component.

https://codesandbox.io/s/9j5ky7zxr

[diasbruno]

@Ne3l I think this is the correct behavior on react@16 and a issue on react-modal. Since your second modal is a children of the first, the events are flowing correctly. There are two options to fix this, we could stop propagating the event if the event is from the overlay or you can move the second modal to be a sibling of the first.

[diasbruno]

It should cause no warm to include a check like event.target == this.overlay on handleOverlayOnClick though.

[Ne3l]

The option of moving the modal to be sibling is not plausible on real world usage. In my case inside the modal i render something like this.

<Modal>
    <List>
        <Item>
            <MoreInfoButton /> //Opens the modal.
        </item
    </List>
</Modal>

About the other solutions you mean the user of the library handles the stopPropagation or the library stops the propagation on requestClose ?

Also i'm not sure if i explained properly but clicking on the ModalOverlay does close it properly. The problem comes when you press esc key inside second modal.

[diasbruno]

gee, sorry. I probably read on a rush. :)

[diasbruno]

Just in case, here is an example of how you can archive the same result using siblings https://github.com/reactjs/react-modal/blob/master/examples/basic/multiple_modals/index.js.

[diasbruno]

@Ne3l can you please review and validate PR #586?

[Ne3l]

@diasbruno

About the siblings, my example was a simplification there are more components in the app hierarchy.
Moving many modals to the root and (pass callbacks or connect with redux) i don't think it would scale well.

About a prop for blocking event propagation to reach parent, do you think is a good idea to add that to react-modal or should the responsability of catching event propagation be handled by the parents?

[diasbruno]

[1] That's fine. I wrote a test for your case, than I realize what was the issue (we had something similar about the click on the overlay).

[2] Since this event must be processed on the topmost modal, it should be safe to stop the propagation.

[diasbruno]

Released v3.1.9.

[diasbruno]

Rereleased as v3.1.10.
3.1.9 was not released from master branch.