Modal doesn't close on the first click on the overlay after click on elements containing event.stopPropagation() inside modal

[ErikDeviashin]

Summary:

When opening a second modal from within a first modal, closing the second modal results in the first modal not closing on the first click on the overlay. Instead, it requires a second click to close. This issue is similar to issue #958. However, the solution provided there is not really suitable for our cross-platform project, which uses components from react-native, and I would like to avoid creating separate button components for the web modal, in which event.stopPropagation() would be removed, in order to achieve the correct behavior.

Steps to reproduce:

1. Open the provided code sandbox: https://codesandbox.io/s/react-modal-layout-issue-xgfsvk
2. Click on the "Open Modal" button to open the first modal.
3. Click on the icon in the top left corner of the first modal to open the second modal.
4. Close the second modal by clicking on the overlay or close button.
5. Attempt to close the first modal by clicking on the overlay.

Expected behavior:

The first modal should close after the first click on the overlay.

Link to example of issue:

https://codesandbox.io/s/react-modal-layout-issue-xgfsvk

Additional notes:

This issue also occurs when closing a modal by clicking on the close button, then reopening it and attempting to close it by clicking on the overlay. The modal only closes after the second click, not the first. This is a kind of standard behavior, so you can reproduce the problem for both the first and second modals. Below are the steps to reproduce the problem for the first modal.

Steps to reproduce:

1. Open the provided code sandbox: https://codesandbox.io/s/react-modal-layout-issue-xgfsvk
2. Click on the "Open Modal" button to open the first modal.
3. Click on the icon in the top right corner of the first modal to close it.
4. Click on the "Open Modal" button again to open the first modal.
5. Attempt to close the first modal by clicking on the overlay.

[ErikDeviashin]

I would like to propose a simple solution to this issue:

In the handleOverlayOnMouseDown method (src/components/ModalPortal.js:321), add this.shouldClose = null; to allow the modal to close in cases where the mouse press started on the overlay. Then, in the handleContentOnMouseDown method (src/components/ModalPortal.js:331), add event.stopPropagation(); as otherwise, there's an issue where the modal would close if OnMouseDown was called within the modal content, but OnMouseUp was called in the layout. I wanted to create a PR with these changes, but I don't have the necessary permissions.

Both of these fixes seem logical to me – if a user clicks on the layout, they likely want to close the modal. Propagation through the content to the layout doesn't seem to provide any benefits, and it appears reasonable not to trigger layout events when clicking on the content.

I would also like to note that I have already applied these changes to our project via a patch, and after testing, our QA reported that the issue was resolved, and no new issues were discovered.

[msubham193]

@ErikDeviashin can i solve this issue ?

[ErikDeviashin]

@msubham193 If you have a right to push in repo, of course you can.
@diasbruno Can you have a look to this issue please?

[diasbruno]

@msubham193 go for it! Let me know if you need anything.

[ErikDeviashin]

@msubham193 Good day! Have you seen my comments in the PR? Can you please correct what is indicated in the comments?

[ErikDeviashin]

@diasbruno Ok, his no react 5 days already, what should I do to make this small fix? It's already third issue with same problem, I really want to solve it and move on

[khmelevskii]

I faced with the same problem. Do we have any workaround before fixing it?

[evoyy]

I have the same problem. In my case I don't have a second modal but I have a drag handler inside my modal content and when I trigger a drag event using the mouse it then requires two clicks on the overlay to close the modal.

I haven't looked into it deeply but I guess it's the same handleOverlayOnMouseDown / handleContentOnMouseDown issue reported by @ErikDeviashin above.

[diasbruno]

From what I saw, the issue actually is that either (or both) components - overlay and content, are receiving an event that is not theirs and they are setting up the flags incorrectly.

The ideal solution is to only act if the event has the event.currentTarget = overlay or content.

[diasbruno]

I've assigned to me to test the lateral buttons...but now I can't remove.
This issue is free to anyone who want to work on it.