Skip to content
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

Modal does not close after first click on the overlay #958

Open
VladBrok opened this issue Jul 27, 2022 · 16 comments
Open

Modal does not close after first click on the overlay #958

VladBrok opened this issue Jul 27, 2022 · 16 comments

Comments

@VladBrok
Copy link

Summary:

Modal ignores the first click on the overlay and closes only after a second click

Steps to reproduce:

  1. Open code sandbox (https://codesandbox.io/s/heuristic-goodall-n25g2w?file=/src/App.js) on the chrome browser (version 103)
  2. Click on the input field
  3. After modal opens, click on the overlay

Expected behavior:

Modal closes after the first click on the overlay

Link to example of issue:

https://codesandbox.io/s/heuristic-goodall-n25g2w?file=/src/App.js

Additional notes:

I've noticed that if I set 'top: 40px' to the 'modal' class, everything works as expected

@jacktan165
Copy link

I'm having the same issue too. I have to click on the overlay twice if you click on the modal content once for some weird reason.

@jacktan165
Copy link

If you look at the source code, it seems like it was done on purpose.

  handleContentOnMouseDown = () => {
    this.shouldClose = false;
  };
  
    handleOverlayOnClick = event => {
    if (this.shouldClose === null) {
      this.shouldClose = true;
    }

    if (this.shouldClose && this.props.shouldCloseOnOverlayClick) {
    ...

No idea why though, I hope a maintainer is reading this and provide some input as to why.

@jacktan165
Copy link

@diasbruno hey, just wondering if you can input on my previous comment, thanks :)

@diasbruno
Copy link
Collaborator

Sure, @jacktan165. Unfortunately, this is weird...the code only fails on chrome. Can you, @VladBrok and @jacktan165, check if this code works on older versions of chrome?

@VladBrok
Copy link
Author

No, sorry, I don't have older chrome versions

@jacktan165
Copy link

@diasbruno I'm currently working on a project with a different codebase but it still have this issue. I tried out that codesandbox @VladBrok has linked on Edge and Firefox, as well as an older Chrome version 79, it's still the same issue.

I don't think it's the browser issue, but specifically related to that variable this.shouldClose. It seems like it is done on purpose. I will look at the git history to see who made that change.

@diasbruno
Copy link
Collaborator

It's quiet strange because I don't recall anyone changing shouldClose otherwise this issue would've opened sooner...
I'm using firefox and chrome (both latest) and it only failed in chrome.

Currently, I don't have much time to work on this project, but it should be a really easy debug to do...so, let me know if you need any help with this. Just ping me.

Good luck!

@diasbruno
Copy link
Collaborator

This piece is a 3 state null, true and false. When click on the content, it sets to false since it's in the hierarchy and it would trigger the click on the overlay (there are more cases for this like "clicking on the content and release it over the overlay").

@diasbruno
Copy link
Collaborator

diasbruno commented Sep 11, 2022

To better debug this, you can use run the examples and set breakpoints on the handles...this way you can see the internal state in the moment of the event.

Are you using react >16?

@jacktan165
Copy link

Oops sorry!
I deleted my previous comment because I realized I got the whole thing wrong, didn't realized you read my message already.

Yeah I'm using React v17 atm. I'm currently debugging my own project atm, and it looks I might be able to figure out the root cause! I have a navigation tab inside the modal, but when I click on one of the navigation tab, it does not trigger handleContentOnMouseDown at all! No idea why, but I am looking into it. If you click somewhere else then the whole process works.

@jacktan165
Copy link

Ah, it did trigger _this.handleContentOnMouseUp, but it is not triggering handleOverlayOnClick to reset this.shouldClose... this is the reason. Now I need to figure out why... initially I thought because my navigation tab is position: sticky, I changed it to static but it's still the same thing.

@diasbruno
Copy link
Collaborator

Try to prevent the event propagation on your component...

@jacktan165
Copy link

I fixed it! I already have event.stopPropagation() on all my onClick, but the solution is to actually REMOVE them. The event.stopPropagation() is preventing the overlay from being clicked.

@diasbruno
Copy link
Collaborator

Your component can go out of the modal's content area (over the overlay)?

@jacktan165
Copy link

No it can't. I have this annoying requirement where I have a button on top of a clickable card. Initially I set event.stopPropagation() on the button so the clickable card is not clicked as well, but that leads to this issue where it ignores the first click when you click on the overlay.

@see2ever
Copy link

see2ever commented Dec 10, 2024

I fixed it! I already have event.stopPropagation() on all my onClick, but the solution is to actually REMOVE them. The event.stopPropagation() is preventing the overlay from being clicked.

it's right, but the overlay element do has some propogation problem.

my solution:

const renderOverlay = ({ onClick, ...rest }: any, children: ReactNode) => {
  const handleClick = (e: React.MouseEvent<HTMLDivElement>) => {
    e.stopPropagation();
    onClick(e);
  };

  return (
    <div {...rest} onClick={handleClick}>
      {children}
    </div>
  );
};
<RcModal
      className={cx(style.commonModal, className)}
      isOpen={on}
      overlayElement={renderOverlay}>
...
</RcModal>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants