Dialog onClose executes twice in mobile

[aldabil21]

Package & version
@headlessui/react 1.7.16

What browser are you using?
Tried on Chrome, Safari, FireFox

Reproduction URL

• Visit https://codesandbox.io/s/awesome-khorana-6gncyz with mobile
• Open the dialog & click outside
• Log "closed" is fired twice

Describe your issue
On mobile, when click outside, the onClose is fired twice.
I've found a related issue & fix #1490, it seems to be still the case on mobile

[aldabil21]

Current workaround, adding

onTouchEnd={(e) => {
 e.preventDefault();
}}

[DavideFrancescon]

I might have found the issue and how to fix it, but I don't know if it's a viable fix. @thecrypticace are you still working on this? If yes, we can talk a bit about it.

[thecrypticace]

@DavideFrancescon yeah I'm literally chatting with @reinink about this right now 😂

What did you have in mind? I'm curious to know.

[DavideFrancescon]

@thecrypticace @reinink I noticed that on mobile both touchEnd and click events get fired, so a workaround was to add a check on the click event with the isMobile function.
don't let vsCode import it or it wont build properly
import { isMobile } from '../utils/platform'

useDocumentEvent(
    'click',
    (event) => {
      if (isMobile()) {
        return
      }
      if (!initialClickTarget.current) {
        return
      }
      handleOutsideClick(event, () => {
        return initialClickTarget.current as HTMLElement
      })
      initialClickTarget.current = null
    },

    // We will use the `capture` phase so that layers in between with `event.stopPropagation()`
    // don't "cancel" this outside click check. E.g.: A `Menu` inside a `DialogPanel` if the `Menu`
    // is open, and you click outside of it in the `DialogPanel` the `Menu` should close. However,
    // the `DialogPanel` has a `onClick(e) { e.stopPropagation() }` which would cancel this.
    true
  )

I tried testing a yarn test and it gets the same results as before the changes, so if that's to be believed, it should work properly everywhere (hopefully)

[thecrypticace]

That is a much better solution than what I'd just come up with. Fantastic. Thank you!

I'll try to get this taken care of today!

[DavideFrancescon]

Happy to be of help, if you want I can do the PR myself and then you can merge it

[thecrypticace]

@DavideFrancescon that'd be great

[thecrypticace]

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

You can try it using out insiders build (may take a few minutes to publish):

• npm install @headlessui/react@insiders
• npm install @headlessui/vue@insiders

Thanks for reporting this one. ✨