Skip to content

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

Open

Description

@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.

Activity

  1. ErikDeviashin commented on Apr 9, 2023

    @ErikDeviashin
    Author

    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.

  2. msubham193 commented on Apr 9, 2023

    @msubham193

    @ErikDeviashin can i solve this issue ?

  3. ErikDeviashin commented on Apr 12, 2023

    @ErikDeviashin
    Author

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

  4. diasbruno commented on Apr 12, 2023

    @diasbruno
    Collaborator

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

  5. ErikDeviashin commented on Apr 16, 2023

    @ErikDeviashin
    Author

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

  6. ErikDeviashin commented on Apr 18, 2023

    @ErikDeviashin
    Author

    @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

  7. khmelevskii commented on Jun 17, 2023

    @khmelevskii
    Contributor

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

  8. evoyy commented on Jul 7, 2023

    @evoyy
    Contributor

    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.

  9. diasbruno commented on Jul 7, 2023

    @diasbruno
    Collaborator

    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.

  10. self-assigned this
    on Jul 7, 2023
  11. diasbruno commented on Jul 7, 2023

    @diasbruno
    Collaborator

    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.

  12. removed their assignment
    on Jul 7, 2023
  13. gautamz07 commented on Jul 20, 2023

    @gautamz07

    @diasbruno @ErikDeviashin if i replace your App.js code with :-

    export default function App() {
      const [isModalOpen, setIsModalOpen] = React.useState(false);
      const [isAdditionalModalOpen, setIsAdditionalModalOpen] = React.useState(
        false
      );
    
      function openModal() {
        setIsModalOpen(true);
      }
    
      function openAdditionalModal() {
        setIsAdditionalModalOpen(true);
      }
    
      function closeModal() {
        setIsModalOpen(false);
      }
    
      function closeAdditionalModal() {
        setIsAdditionalModalOpen(false);
      }
    
      return (
        <>
          <button className="openModalBtn" onClick={openModal}>
            Open Modal
          </button>
          <ReactModal
            style={{
              overlay: {
                position: "fixed",
                top: 0,
                left: 0,
                right: 0,
                bottom: 0,
                backgroundColor: "rgba(0, 0, 0, 0.6)",
                display: "flex",
                justifyContent: "center",
                alignItems: "center"
              },
              content: {
                position: "unset",
                background: "transparent",
                outline: "none",
                padding: 0,
                border: "none",
                display: "flex",
                justifyContent: "center"
              }
            }}
            isOpen={isModalOpen}
            onRequestClose={closeModal}
            shouldReturnFocusAfterClose={false}
          >
            {/* <TouchableOpacity onPress={openAdditionalModal}>
              <img
                className="modalIcon"
                src="modal.png"
                alt="close_btn"
                width="20"
                height="20"
              />
            </TouchableOpacity> */}
            <div 
                className="modalIcon"
                onClick={openAdditionalModal}
              >
                BUG
            </div> 
            <div className="contentWrapper">
               MODAL
               &nbsp;
               &nbsp;
               &nbsp;
               {/* <br /> */}
               {/* <br /> */}
               <button
                onClick={openAdditionalModal}
               >
                The Bug
              </button>
            </div>
            {/* <TouchableOpacity onPress={closeModal}>
              <img
                className="closeIcon"
                src="cross-23.png"
                alt="close_btn"
                width="20"
                height="20"
              />
            </TouchableOpacity> */}
          </ReactModal>
          <ReactModal
            style={{
              overlay: {
                position: "fixed",
                top: 0,
                left: 0,
                right: 0,
                bottom: 0,
                backgroundColor: "transparent",
                display: "flex",
                justifyContent: "center",
                alignItems: "center"
              },
              content: {
                position: "unset",
                background: "transparent",
                outline: "none",
                padding: 0,
                border: "none",
                display: "flex",
                justifyContent: "center"
              }
            }}
            isOpen={isAdditionalModalOpen}
            onRequestClose={closeAdditionalModal}
            shouldReturnFocusAfterClose={false}
          >
            <div className="contentWrapper">Additional Modal</div>
            {/* <TouchableOpacity onPress={closeAdditionalModal}>
              <img
                className="closeIcon"
                src="cross-23.png"
                alt="close_btn"
                width="20"
                height="20"
              />
            </TouchableOpacity> */}
          </ReactModal>
        </>
      );
    }
    
    

    The error does't occur , just an observation , i'am trying to pin point the issue here without any react-native components , would appreciate any insights.

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions