From c8b297bfe8607d62d5326350e207593570bf3a94 Mon Sep 17 00:00:00 2001
From: Robin Malfait <malfait.robin@gmail.com>
Date: Wed, 21 Apr 2021 14:13:52 +0200
Subject: [PATCH] fix dialog event propagation (#422)

* re-export the `screen` utility for quick debugging purposes

* stop event propagation when clicking inside a Dialog

Fixes: #414
---
 .../src/components/dialog/dialog.test.tsx     | 70 +++++++++++++++
 .../src/components/dialog/dialog.tsx          | 16 +++-
 .../src/components/dialog/dialog.test.ts      | 86 +++++++++++++++++++
 .../src/components/dialog/dialog.ts           | 11 ++-
 .../src/test-utils/vue-testing-library.ts     |  4 +-
 5 files changed, 183 insertions(+), 4 deletions(-)

diff --git a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx
index f9d4a6bcf6..eb4e7a9282 100644
--- a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx
+++ b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx
@@ -496,4 +496,74 @@ describe('Mouse interactions', () => {
       assertActiveElement(getByText('Hello'))
     })
   )
+
+  it(
+    'should stop propagating click events when clicking on the Dialog.Overlay',
+    suppressConsoleLogs(async () => {
+      let wrapperFn = jest.fn()
+      function Example() {
+        let [isOpen, setIsOpen] = useState(true)
+        return (
+          <div onClick={wrapperFn}>
+            <Dialog open={isOpen} onClose={setIsOpen}>
+              Contents
+              <Dialog.Overlay />
+              <TabSentinel />
+            </Dialog>
+          </div>
+        )
+      }
+      render(<Example />)
+
+      // Verify it is open
+      assertDialog({ state: DialogState.Visible })
+
+      // Verify that the wrapper function has not been called yet
+      expect(wrapperFn).toHaveBeenCalledTimes(0)
+
+      // Click the Dialog.Overlay to close the Dialog
+      await click(getDialogOverlay())
+
+      // Verify it is closed
+      assertDialog({ state: DialogState.InvisibleUnmounted })
+
+      // Verify that the wrapper function has not been called yet
+      expect(wrapperFn).toHaveBeenCalledTimes(0)
+    })
+  )
+
+  it(
+    'should stop propagating click events when clicking on an element inside the Dialog',
+    suppressConsoleLogs(async () => {
+      let wrapperFn = jest.fn()
+      function Example() {
+        let [isOpen, setIsOpen] = useState(true)
+        return (
+          <div onClick={wrapperFn}>
+            <Dialog open={isOpen} onClose={setIsOpen}>
+              Contents
+              <button onClick={() => setIsOpen(false)}>Inside</button>
+              <TabSentinel />
+            </Dialog>
+          </div>
+        )
+      }
+      render(<Example />)
+
+      // Verify it is open
+      assertDialog({ state: DialogState.Visible })
+
+      // Verify that the wrapper function has not been called yet
+      expect(wrapperFn).toHaveBeenCalledTimes(0)
+
+      // Click the button inside the the Dialog
+      await click(getByText('Inside'))
+
+      // Verify it is closed
+      assertDialog({ state: DialogState.InvisibleUnmounted })
+
+      // Verify that the wrapper function has not been called yet
+      expect(wrapperFn).toHaveBeenCalledTimes(0)
+    })
+  )
 })
diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx
index 32d2fcf9f1..f5388912f4 100644
--- a/packages/@headlessui-react/src/components/dialog/dialog.tsx
+++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx
@@ -92,7 +92,13 @@ let DEFAULT_DIALOG_TAG = 'div' as const
 interface DialogRenderPropArg {
   open: boolean
 }
-type DialogPropsWeControl = 'id' | 'role' | 'aria-modal' | 'aria-describedby' | 'aria-labelledby'
+type DialogPropsWeControl =
+  | 'id'
+  | 'role'
+  | 'aria-modal'
+  | 'aria-describedby'
+  | 'aria-labelledby'
+  | 'onClick'
 
 let DialogRenderFeatures = Features.RenderStrategy | Features.Static
 
@@ -176,6 +182,8 @@ let DialogRoot = forwardRefWithAs(function Dialog<
     if (event.key !== Keys.Escape) return
     if (dialogState !== DialogStates.Open) return
     if (containers.current.size > 1) return // 1 is myself, otherwise other elements in the Stack
+    event.preventDefault()
+    event.stopPropagation()
     close()
   })
 
@@ -243,6 +251,10 @@ let DialogRoot = forwardRefWithAs(function Dialog<
     'aria-modal': dialogState === DialogStates.Open ? true : undefined,
     'aria-labelledby': state.titleId,
     'aria-describedby': describedby,
+    onClick(event: ReactMouseEvent) {
+      event.preventDefault()
+      event.stopPropagation()
+    },
   }
   let passthroughProps = rest
 
@@ -302,6 +314,8 @@ let Overlay = forwardRefWithAs(function Overlay<
   let handleClick = useCallback(
     (event: ReactMouseEvent) => {
       if (isDisabledReactIssue7711(event.currentTarget)) return event.preventDefault()
+      event.preventDefault()
+      event.stopPropagation()
       close()
     },
     [close]
diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts
index 07bfe0d43f..ec91b014ab 100644
--- a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts
+++ b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts
@@ -607,4 +607,90 @@ describe('Mouse interactions', () => {
       assertActiveElement(getByText('Hello'))
     })
   )
+
+  it(
+    'should stop propagating click events when clicking on the Dialog.Overlay',
+    suppressConsoleLogs(async () => {
+      let wrapperFn = jest.fn()
+      renderTemplate({
+        template: `
+          <div @click="wrapperFn">
+            <Dialog v-if="true" :open="isOpen" @close="setIsOpen">
+              Contents
+              <DialogOverlay />
+              <TabSentinel />
+            </Dialog>
+          </div>
+        `,
+        setup() {
+          let isOpen = ref(true)
+          return {
+            isOpen,
+            wrapperFn,
+            setIsOpen(value: boolean) {
+              isOpen.value = value
+            },
+          }
+        },
+      })
+
+      // Verify it is open
+      assertDialog({ state: DialogState.Visible })
+
+      // Verify that the wrapper function has not been called yet
+      expect(wrapperFn).toHaveBeenCalledTimes(0)
+
+      // Click the Dialog.Overlay to close the Dialog
+      await click(getDialogOverlay())
+
+      // Verify it is closed
+      assertDialog({ state: DialogState.InvisibleUnmounted })
+
+      // Verify that the wrapper function has not been called yet
+      expect(wrapperFn).toHaveBeenCalledTimes(0)
+    })
+  )
+
+  it(
+    'should stop propagating click events when clicking on an element inside the Dialog',
+    suppressConsoleLogs(async () => {
+      let wrapperFn = jest.fn()
+      renderTemplate({
+        template: `
+          <div @click="wrapperFn">
+            <Dialog v-if="true" :open="isOpen" @close="setIsOpen">
+              Contents
+              <button @click="setIsOpen(false)">Inside</button>
+              <TabSentinel />
+            </Dialog>
+          </div>
+        `,
+        setup() {
+          let isOpen = ref(true)
+          return {
+            isOpen,
+            wrapperFn,
+            setIsOpen(value: boolean) {
+              isOpen.value = value
+            },
+          }
+        },
+      })
+
+      // Verify it is open
+      assertDialog({ state: DialogState.Visible })
+
+      // Verify that the wrapper function has not been called yet
+      expect(wrapperFn).toHaveBeenCalledTimes(0)
+
+      // Click the button inside the the Dialog
+      await click(getByText('Inside'))
+
+      // Verify it is closed
+      assertDialog({ state: DialogState.InvisibleUnmounted })
+
+      // Verify that the wrapper function has not been called yet
+      expect(wrapperFn).toHaveBeenCalledTimes(0)
+    })
+  )
 })
diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.ts b/packages/@headlessui-vue/src/components/dialog/dialog.ts
index 79c379ecc5..0e73eb62da 100644
--- a/packages/@headlessui-vue/src/components/dialog/dialog.ts
+++ b/packages/@headlessui-vue/src/components/dialog/dialog.ts
@@ -85,6 +85,7 @@ export let Dialog = defineComponent({
       'aria-modal': this.dialogState === DialogStates.Open ? true : undefined,
       'aria-labelledby': this.titleId,
       'aria-describedby': this.describedby,
+      onClick: this.handleClick,
     }
     let { open, initialFocus, ...passThroughProps } = this.$props
     let slot = { open: this.dialogState === DialogStates.Open }
@@ -188,6 +189,8 @@ export let Dialog = defineComponent({
       if (event.key !== Keys.Escape) return
       if (dialogState.value !== DialogStates.Open) return
       if (containers.value.size > 1) return // 1 is myself, otherwise other elements in the Stack
+      event.preventDefault()
+      event.stopPropagation()
       api.close()
     })
 
@@ -241,6 +244,10 @@ export let Dialog = defineComponent({
       dialogState,
       titleId,
       describedby,
+      handleClick(event: MouseEvent) {
+        event.preventDefault()
+        event.stopPropagation()
+      },
     }
   },
 })
@@ -276,7 +283,9 @@ export let DialogOverlay = defineComponent({
 
     return {
       id,
-      handleClick() {
+      handleClick(event: MouseEvent) {
+        event.preventDefault()
+        event.stopPropagation()
         api.close()
       },
     }
diff --git a/packages/@headlessui-vue/src/test-utils/vue-testing-library.ts b/packages/@headlessui-vue/src/test-utils/vue-testing-library.ts
index d3b596ac78..81eb7bbb66 100644
--- a/packages/@headlessui-vue/src/test-utils/vue-testing-library.ts
+++ b/packages/@headlessui-vue/src/test-utils/vue-testing-library.ts
@@ -1,5 +1,5 @@
 import { mount } from '@vue/test-utils'
-import { logDOM, fireEvent } from '@testing-library/dom'
+import { logDOM, fireEvent, screen } from '@testing-library/dom'
 
 let mountedWrappers = new Set()
 
@@ -58,4 +58,4 @@ if (typeof afterEach === 'function') {
   afterEach(() => cleanup())
 }
 
-export { fireEvent }
+export { fireEvent, screen }