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

Allow to click on elements inside a Dialog Overlay #816

Merged
merged 2 commits into from
Sep 17, 2021

Conversation

RobinMalfait
Copy link
Collaborator

@RobinMalfait RobinMalfait commented Sep 17, 2021

Some users added content directly to the Dialog.Overlay, but we have a click handler enabled so that if you click on the Dialog.Overlay that it closes the Dialog.

With this PR we make sure to check if we clicked on the Dialog.Overlay only (and not on any of its children).

Fixes: #812

@vercel
Copy link

vercel bot commented Sep 17, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

headlessui-vue – ./packages/@headlessui-vue

🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-vue/8itD3RuVzsN2Du2qQEfYPrPWvxWP
✅ Preview: https://headlessui-vue-git-dialog-overlay-click-bug-tailwindlabs.vercel.app

headlessui-react – ./packages/@headlessui-react

🔍 Inspect: https://vercel.com/tailwindlabs/headlessui-react/8KKSPvB6SFebWhZDupg1zMwt2hgo
✅ Preview: https://headlessui-react-git-dialog-overlay-click-bug-tailwindlabs.vercel.app

@RobinMalfait RobinMalfait changed the title allow to click on elements inside a Dialog Overlay Allow to click on elements inside a Dialog Overlay Sep 17, 2021
@marcobusemann
Copy link

marcobusemann commented Jan 10, 2022

Hey, I stumbled up on this as our Dialogs are now not closing anymore when clicking on the backdrop.
Our code looks like this:

    <Dialog.Overlay className="fixed inset-0">
      <div
        className={`absolute inset-0 bg-airblue opacity-50 ${className || ''}`}
      />
    </Dialog.Overlay>

So this is basically a breaking change.
If I remember correctly, I took this from Tailwind-UI a several month ago.

@RobinMalfait
Copy link
Collaborator Author

RobinMalfait commented Jan 10, 2022

@marcobusemann thanks for the bug report.

I double checked version control (internal Tailwind UI reference: May 22, 2021 - PR 342) (you won't be able to see it since it's a private repo, but if a colleague of mine stumbles upon it, then they can look at the relevant PR as well)), and when we introduced React/Vue components we didn't have a nested child in the Dialog.Overlay.

In your case, it should be possible to change it to:

<Dialog.Overlay className={`fixed inset-0 bg-airblue opacity-50 ${className || ''}`} />

Does that work for you?

@marcobusemann
Copy link

Then I am not sure why we ended up with this child element.
Yes that works (with bg-opacity instead of opacity when used with Transition).
Maybe you should add a breaking-change notice to the release-notes.

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

Successfully merging this pull request may close these issues.

None yet

2 participants