tailwindlabs / headlessui

Completely unstyled, fully accessible UI components, designed to integrate beautifully with Tailwind CSS.
https://headlessui.com
MIT License
25.73k stars 1.06k forks source link

[Bug]: Interacting with third-party components that use portals inside a Dialog closes the Dialog #432

Closed bakerkretzmar closed 2 years ago

bakerkretzmar commented 3 years ago

What package within Headless UI are you using?

@headlessui/vue

What version of that package are you using?

1.0.0

What browser are you using?

Firefox

Reproduction repository

https://codesandbox.io/s/pensive-fast-yxokp?file=/src/App.vue

Describe your issue

When any component inside a Dialog renders itself elsewhere in the DOM using a Portal/Teleport, clicking in it closes the entire Dialog. The example above uses vue-flatpickr-component to create a date picker inside a modal, but because it renders the actual flatpickr instance somewhere else in the DOM, clicking any date closes the entire modal.

The docs on this say:

When a Dialog is rendered, clicking the DialogOverlay will close the Dialog.

But that isn't exactly the case—it's not clicking the DialogOverlay specifically that closes the Dialog, but clicking anything that is not a child of the Dialog in the DOM.

I believe the relevant part of the code is here: https://github.com/tailwindlabs/headlessui/blob/main/packages/%40headlessui-vue/src/components/dialog/dialog.ts#L174-L184

What do you think about adding a way to disable that global mousedown handler, or adding an option to actually only close the Dialog based on clicks directly on the DialogOverlay component?

RomainLanz commented 3 years ago

I have the same issue as described, clicking on the calendar provided by flatpickr closes the Dialog.

ThatOneAwkwardGuy commented 3 years ago

I Also have the same issue with showing a loading indicator above the Modal, any accidental clicks on the loading indicator will then close the modal.

michgeek commented 3 years ago

I can confirm the issue. I use dialog with scrollable content that contains a selectbox. The selectbox options list is rendered at body level in a teleport and positioned with popper. Clicking on the options list will close the dialog.

narciero commented 3 years ago

is there a workaround for this? seeing the same issue with @headlessui/react@1.2.0

bakerkretzmar commented 3 years ago

I was able to get it working on my project for now by also setting static: true in the Flatpickr config, but this issue is still there by default and might not be avoidable with other libraries.

agorin commented 3 years ago

I have the same issue as described, clicking on the calendar provided by devextreme closes the Dialog. is there a workaround for this?

nasa8x commented 3 years ago

I am using CKeditor BalloonEditor the Modal also close after clicking editor toolbar. Plz fix soon. Thank you.

chinanderm commented 3 years ago

What is the fix/solution or status of this? I would really like to use headless UI for dialogs and other things but cannot if it breaks my existing usage of third party libraries such as flatpickr.

loktarjugg commented 3 years ago

i have the same issue. when i use some element over the dialog and click the element, dialog will be close

Aleksion commented 3 years ago

What about changing the

 useWindowEvent('mousedown', function (event) {
    var _internalDialogRef$cu;

    var target = event.target;
    if (dialogState !== DialogStates.Open) return;
    if (hasNestedDialogs) return;
    if ((_internalDialogRef$cu = internalDialogRef.current) == null ? void 0 : _internalDialogRef$cu.contains(target)) return;
    close();
  }); // Scroll lock

to a click handler on the overlay instead?

agorin commented 3 years ago

Any news about the fix?

RomainLanz commented 2 years ago

Hey there! 👋🏻

Small bump of the issue.

dcastil commented 2 years ago

I opened a PR to fix the issue for the React-specific version in https://github.com/tailwindlabs/headlessui/pull/989. Not sure though whether this fix is also applicable in Vue since it relies on the event delegation system of React which also propagates events through portals.

RodolfoSilva commented 2 years ago

I'm using react. And this comment solve my issue.

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

You can import the Portal component from Headless UI, and wrap the Toast/notification into a Portal component. When a Portal component is used inside a Dialog then we will render the contents in the root of the Dialog instead of the root of the document.

https://github.com/tailwindlabs/headlessui/issues/780#issuecomment-917663939

Thanks @RobinMalfait

nasa8x commented 2 years ago

With React just add this it worked for me: <Dialog as="div" static onClose={() => null} className="fixed z-50 inset-0 overflow-y-auto modal">

agorin commented 2 years ago

With Vue, it does not work...

RomainLanz commented 2 years ago

With React just add this it worked for me: <Dialog as="div" static onClose={() => null} className="fixed z-50 inset-0 overflow-y-auto modal">

This could work. The issue is now with other way to close the dialog.

The quick solution would be to remove the @close handler on the Dialog component and adding @click handler on the DialogOverlay to emit the close event.

You still need to handle other way of closing the dialog, like pressing the Escape key.

I don't know if there are other things I am missing, here is an example.

<Dialog as="div" [...] @keydown.esc="$emit('close')" static>
  <DialogOverlay [...] @click="$emit('close')" />
</Dialog>
RomainLanz commented 2 years ago

After testing the solution I have proposed, there are still issues with flatpickr. Somehow, the year input is not focusable when it is inside the Dialog component.

eskiesirius commented 2 years ago

any workaround in vue?

sgehrman commented 2 years ago

same bug. What's taking so long?

RobinMalfait commented 2 years ago

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

This should be fixed #1268, and will be available in the next release.

You can already try it using:


I've updated your CodeSandbox with the latest insiders build: https://codesandbox.io/s/sweet-tree-egth6u

bakerkretzmar commented 2 years ago

Thanks! That demo works for me, I have a few questions though that I'll leave as comments on the PR.

sgehrman commented 2 years ago

steve@5950x:~/Documents/GitHub/mavrik/nftcompany-frontend$ npm install @headlessui/react@insiders npm ERR! Cannot read properties of null (reading 'matches')

npm ERR! A complete log of this run can be found in: npm ERR! /home/steve/.npm/_logs/2022-03-25T01_09_21_535Z-debug-0.log

NEVERMIND: apparently, I'm on this wacky project and had to

npx pnpm install @headlessui/react@insiders

This is why I'm a Flutter dev. JS is non stop weird bugs.

eskiesirius commented 2 years ago

It worked but there is a bug with @vuepic/vue-datepicker

https://codesandbox.io/s/exciting-fog-cefyoh?file=/src/App.vue

RobinMalfait commented 2 years ago

@eskiesirius do you mind telling me what the bug is? 😅

image
eskiesirius commented 2 years ago

@eskiesirius do you mind telling me what the bug is? 😅

image

sorry but it opens automatically

RobinMalfait commented 2 years ago

@eskiesirius that's because the library is implemented to open when it receives focus. And the Dialog component focuses the first focusable item. If you don't want this behaviour: https://headlessui.dev/vue/dialog#managing-focus-within-your-dialog

eskiesirius commented 2 years ago

okay.. thank you so much!!! can you refer this to my issue i posted here?

vinnygambiny commented 1 year ago

I still have an issue related to the portal and the Popover with Combobox.

I use headlessui-float with the portal, and when the focus goes to the ComboboxInput, the Popover close. If I remove the ComboboxInput, then it's when I click on an ComboboxOption.

Here is an exemple https://codesandbox.io/s/billowing-thunder-0s724d

sbc640964 commented 1 year ago

I have the opposite problem. I render a listbox in a portal from a dialog, and cannot create a focus in it... (because it is outside the portal of the dialog)