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

[Bug]: Clicking elements outside Dialogs closes dialog #1138

Closed
michael-vasyliv opened this issue Feb 23, 2022 · 7 comments
Closed

[Bug]: Clicking elements outside Dialogs closes dialog #1138

michael-vasyliv opened this issue Feb 23, 2022 · 7 comments
Assignees

Comments

@michael-vasyliv
Copy link

michael-vasyliv commented Feb 23, 2022

What package within Headless UI are you using?
@headlessui/react

What version of that package are you using?
1.5.0

What browser are you using?
Chrome

Reproduction URL
https://n797o6.csb.app/

Describe your issue
Scenario: We have a HeadlessUI Dialog and react-toastify. When a dialog is open we click on a notification to close it but it also closes the dialog.
In my opinion, it will be easy to remove/disable this listener and just use this

<Dialog.Overlay className="fixed inset-0 bg-black bg-opacity-50" onClick={closeThisDialogWithoutAnyFckPain} />

https://github.com/tailwindlabs/headlessui/blob/main/packages/%40headlessui-react/src/components/dialog/dialog.tsx#L210

I've tried the solution with Portal but it didn't help

Related issues
#780
#432

@michael-vasyliv michael-vasyliv changed the title [Bug]: Clicking elements above Dialogs closes dialog [Bug]: Clicking elements outside Dialogs closes dialog Feb 23, 2022
@RobinMalfait RobinMalfait self-assigned this Feb 24, 2022
@RobinMalfait
Copy link
Collaborator

Hey! Thank you for your bug report!
Much appreciated! 🙏

The Portal solution only works if you are in control of rendering the toast yourself AND if you render the Portal as part of the Dialog. Here is an updated codesandbox: https://codesandbox.io/s/lucid-hugle-vn8w52?file=/src/App.js

The Portal itself will still render it in the DOM in a portal root right underneath the body. But the nesting is important from React's perspective so that we can look at the React tree and communicate between the components via context.

@michael-vasyliv
Copy link
Author

@RobinMalfait thank you for the idea but it does not help for a "real project", the problem is that basically ToastContainer is placed in the root app component while the Dialog is placed somewhere in deep, so adding ToastContainer for each Dialog is not a good solution.
So how can I disable https://github.com/tailwindlabs/headlessui/blob/main/packages/%40headlessui-react/src/components/dialog/dialog.tsx#L210

@aquaticone
Copy link

I'd also like to be able to disable the mousedown event handler.

There are a number of scenarios in which this global handling is problematic, including toasts.

@SpiritusDeos
Copy link

SpiritusDeos commented Mar 10, 2022

Hello 👋🏻 , any news on this issue ?

I have the same problem using the date-picker in element-plus, clicking on the top right corner of the date-picker trigger a closed on the dialog

@peterKaleta
Copy link

@RobinMalfait I'll agree with @michael-vasyliv here. The <Portal> solution is not really usable across bigger projects with generic use of react-toastify. The mousedown/implicit passing of closing method sounds good.

@connorlindsey
Copy link

Was running into the same problem except for a custom notification/toast component and found a solution that's working well for my app. I added a check to the onClose function and check if the notification component was clicked. In my case, I had to check the click events path to get it working.

Depending on your app, you'll need to change the if statement, but hopefully it gives you an idea of how to solve this while still retaining the nice parts of detecting click outside.

<Dialog ...
  onClose={(val) => {
    // Don't close if clicked on a notification
    if (event.path.some((elem) => elem.classList?.contains("notification"))) {
      return null
    }
    setOpen(val)
  }
...></Dialog>

@Aravinda93
Copy link

@connorlindsey @RobinMalfait
I am facing the similar issue in my Headless UI Vue, can you please have a look and provide some solution?
Issue: #3199

Minimal reproduction repo in CodeSandBox

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

7 participants