Fix incorrect closing while interacting with third party libraries in…

… `Dialog` component (#1268) * ensure to keep the Dialog open when clicking on 3rd party elements * update playground with a Flatpickr example * update changelog
tailwindlabs · Mar 24, 2022 · c1023f7 · c1023f7
1 parent 3e19aa5
commit c1023f7
Show file tree

Hide file tree

Showing 14 changed files with 218 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Stop propagation on the Popover Button ([#1263](https://github.com/tailwindlabs/headlessui/pull/1263))
 - Fix incorrect `active` option in the Listbox/Combobox component ([#1264](https://github.com/tailwindlabs/headlessui/pull/1264))
 - Properly merge incoming props ([#1265](https://github.com/tailwindlabs/headlessui/pull/1265))
+- Fix incorrect closing while interacting with third party libraries in `Dialog` component ([#1268](https://github.com/tailwindlabs/headlessui/pull/1268))
 
 ### Added
 
@@ -64,6 +65,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Improve Combobox Input value ([#1248](https://github.com/tailwindlabs/headlessui/pull/1248))
 - Fix Tree-shaking support ([#1247](https://github.com/tailwindlabs/headlessui/pull/1247))
 - Stop propagation on the Popover Button ([#1263](https://github.com/tailwindlabs/headlessui/pull/1263))
+- Fix incorrect closing while interacting with third party libraries in `Dialog` component ([#1268](https://github.com/tailwindlabs/headlessui/pull/1268))
 
 ### Added
 

diff --git a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx
@@ -19,6 +19,7 @@ import {
 import { click, press, Keys } from '../../test-utils/interactions'
 import { PropsOf } from '../../types'
 import { Transition } from '../transitions/transition'
+import { createPortal } from 'react-dom'
 
 jest.mock('../../hooks/use-id')
 
@@ -843,6 +844,53 @@ describe('Mouse interactions', () => {
       assertDialog({ state: DialogState.Visible })
     })
   )
+
+  it(
+    'should be possible to click on elements created by third party libraries',
+    suppressConsoleLogs(async () => {
+      let fn = jest.fn()
+      function ThirdPartyLibrary() {
+        return createPortal(
+          <>
+            <button data-lib onClick={fn}>
+              3rd party button
+            </button>
+          </>,
+          document.body
+        )
+      }
+
+      function Example() {
+        let [isOpen, setIsOpen] = useState(true)
+
+        return (
+          <div>
+            <span>Main app</span>
+            <Dialog open={isOpen} onClose={setIsOpen}>
+              <div>
+                Contents
+                <TabSentinel />
+              </div>
+            </Dialog>
+            <ThirdPartyLibrary />
+          </div>
+        )
+      }
+      render(<Example />)
+
+      // Verify it is open
+      assertDialog({ state: DialogState.Visible })
+
+      // Click the button inside the 3rd party library
+      await click(document.querySelector('[data-lib]'))
+
+      // Verify we clicked on the 3rd party button
+      expect(fn).toHaveBeenCalledTimes(1)
+
+      // Verify the dialog is still open
+      assertDialog({ state: DialogState.Visible })
+    })
+  )
 })
 
 describe('Nesting', () => {

diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx
@@ -200,7 +200,7 @@ let DialogRoot = forwardRefWithAs(function Dialog<
   // in between. We only care abou whether you are the top most one or not.
   let position = !hasNestedDialogs ? 'leaf' : 'parent'
 
-  useFocusTrap(
+  let previousElement = useFocusTrap(
     internalDialogRef,
     enabled
       ? match(position, {
@@ -213,12 +213,26 @@ let DialogRoot = forwardRefWithAs(function Dialog<
   useInertOthers(internalDialogRef, hasNestedDialogs ? enabled : false)
 
   // Handle outside click
-  useOutsideClick(internalDialogRef, () => {
-    if (dialogState !== DialogStates.Open) return
-    if (hasNestedDialogs) return
+  useOutsideClick(
+    () => {
+      // Third party roots
+      let rootContainers = Array.from(ownerDocument?.querySelectorAll('body > *') ?? []).filter(
+        (container) => {
+          if (!(container instanceof HTMLElement)) return false // Skip non-HTMLElements
+          if (container.contains(previousElement.current)) return false // Skip if it is the main app
+          return true // Keep
+        }
+      )
 
-    close()
-  })
+      return [...rootContainers, internalDialogRef.current] as HTMLElement[]
+    },
+    () => {
+      if (dialogState !== DialogStates.Open) return
+      if (hasNestedDialogs) return
+
+      close()
+    }
+  )
 
   // Handle `Escape` to close
   useEventListener(ownerDocument?.defaultView, 'keydown', (event) => {

diff --git a/packages/@headlessui-react/src/hooks/use-focus-trap.ts b/packages/@headlessui-react/src/hooks/use-focus-trap.ts
@@ -152,6 +152,8 @@ export function useFocusTrap(
     },
     true
   )
+
+  return restoreElement
 }
 
 function contains(containers: Set<MutableRefObject<HTMLElement | null>>, element: HTMLElement) {

diff --git a/packages/@headlessui-react/src/hooks/use-outside-click.ts b/packages/@headlessui-react/src/hooks/use-outside-click.ts
@@ -17,26 +17,14 @@ function microTask(cb: () => void) {
   }
 }
 
+type Container = MutableRefObject<HTMLElement | null> | HTMLElement | null
+type ContainerCollection = Container[] | Set<Container>
+type ContainerInput = Container | ContainerCollection
+
 export function useOutsideClick(
-  containers:
-    | HTMLElement
-    | MutableRefObject<HTMLElement | null>
-    | (MutableRefObject<HTMLElement | null> | HTMLElement | null)[]
-    | Set<HTMLElement>,
+  containers: ContainerInput | (() => ContainerInput),
   cb: (event: MouseEvent | PointerEvent, target: HTMLElement) => void
 ) {
-  let _containers = useMemo(() => {
-    if (Array.isArray(containers)) {
-      return containers
-    }
-
-    if (containers instanceof Set) {
-      return containers
-    }
-
-    return [containers]
-  }, [containers])
-
   let called = useRef(false)
   let handler = useLatestValue((event: MouseEvent | PointerEvent) => {
     if (called.current) return
@@ -45,6 +33,22 @@ export function useOutsideClick(
       called.current = false
     })
 
+    let _containers = (function resolve(containers): ContainerCollection {
+      if (typeof containers === 'function') {
+        return resolve(containers())
+      }
+
+      if (Array.isArray(containers)) {
+        return containers
+      }
+
+      if (containers instanceof Set) {
+        return containers
+      }
+
+      return [containers]
+    })(containers)
+
     let target = event.target as HTMLElement
 
     // Ignore if the target doesn't exist in the DOM anymore

diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts
@@ -1035,6 +1035,59 @@ describe('Mouse interactions', () => {
       assertDialog({ state: DialogState.Visible })
     })
   )
+
+  it(
+    'should be possible to click on elements created by third party libraries',
+    suppressConsoleLogs(async () => {
+      let fn = jest.fn()
+
+      let ThirdPartyLibrary = defineComponent({
+        template: html`
+          <teleport to="body">
+            <button data-lib @click="fn">3rd party button</button>
+          </teleport>
+        `,
+        setup: () => ({ fn }),
+      })
+
+      renderTemplate({
+        components: { ThirdPartyLibrary },
+        template: `
+          <div>
+            <span>Main app</span>
+            <Dialog :open="isOpen" @close="setIsOpen">
+              <div>
+                Contents
+                <TabSentinel />
+              </div>
+            </Dialog>
+            <ThirdPartyLibrary />
+          </div>
+        `,
+        setup() {
+          let isOpen = ref(true)
+          return {
+            isOpen,
+            setIsOpen(value: boolean) {
+              isOpen.value = value
+            },
+          }
+        },
+      })
+
+      // Verify it is open
+      assertDialog({ state: DialogState.Visible })
+
+      // Click the button inside the 3rd party library
+      await click(document.querySelector('[data-lib]'))
+
+      // Verify we clicked on the 3rd party button
+      expect(fn).toHaveBeenCalledTimes(1)
+
+      // Verify the dialog is still open
+      assertDialog({ state: DialogState.Visible })
+    })
+  )
 })
 
 describe('Nesting', () => {

diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.ts b/packages/@headlessui-vue/src/components/dialog/dialog.ts
@@ -129,7 +129,7 @@ export let Dialog = defineComponent({
     // in between. We only care abou whether you are the top most one or not.
     let position = computed(() => (!hasNestedDialogs.value ? 'leaf' : 'parent'))
 
-    useFocusTrap(
+    let previousElement = useFocusTrap(
       internalDialogRef,
       computed(() => {
         return enabled.value
@@ -191,13 +191,28 @@ export let Dialog = defineComponent({
     provide(DialogContext, api)
 
     // Handle outside click
-    useOutsideClick(internalDialogRef, (_event, target) => {
-      if (dialogState.value !== DialogStates.Open) return
-      if (hasNestedDialogs.value) return
+    useOutsideClick(
+      () => {
+        // Third party roots
+        let rootContainers = Array.from(
+          ownerDocument.value?.querySelectorAll('body > *') ?? []
+        ).filter((container) => {
+          if (!(container instanceof HTMLElement)) return false // Skip non-HTMLElements
+          if (container.contains(previousElement.value)) return false // Skip if it is the main app
+          return true // Keep
+        })
 
-      api.close()
-      nextTick(() => target?.focus())
-    })
+        return [...rootContainers, internalDialogRef.value] as HTMLElement[]
+      },
+
+      (_event, target) => {
+        if (dialogState.value !== DialogStates.Open) return
+        if (hasNestedDialogs.value) return
+
+        api.close()
+        nextTick(() => target?.focus())
+      }
+    )
 
     // Handle `Escape` to close
     useEventListener(ownerDocument.value?.defaultView, 'keydown', (event) => {

diff --git a/packages/@headlessui-vue/src/hooks/use-focus-trap.ts b/packages/@headlessui-vue/src/hooks/use-focus-trap.ts
@@ -175,6 +175,8 @@ export function useFocusTrap(
     },
     true
   )
+
+  return restoreElement
 }
 
 function contains(containers: Set<Ref<HTMLElement | null>>, element: HTMLElement) {

diff --git a/packages/@headlessui-vue/src/hooks/use-outside-click.ts b/packages/@headlessui-vue/src/hooks/use-outside-click.ts
@@ -17,12 +17,12 @@ function microTask(cb: () => void) {
   }
 }
 
+type Container = Ref<HTMLElement | null> | HTMLElement | null
+type ContainerCollection = Container[] | Set<Container>
+type ContainerInput = Container | ContainerCollection
+
 export function useOutsideClick(
-  containers:
-    | HTMLElement
-    | Ref<HTMLElement | null>
-    | (Ref<HTMLElement | null> | HTMLElement | null)[]
-    | Set<HTMLElement>,
+  containers: ContainerInput | (() => ContainerInput),
   cb: (event: MouseEvent | PointerEvent, target: HTMLElement) => void
 ) {
   let called = false
@@ -38,7 +38,11 @@ export function useOutsideClick(
     // Ignore if the target doesn't exist in the DOM anymore
     if (!target.ownerDocument.documentElement.contains(target)) return
 
-    let _containers = (() => {
+    let _containers = (function resolve(containers): ContainerCollection {
+      if (typeof containers === 'function') {
+        return resolve(containers())
+      }
+
       if (Array.isArray(containers)) {
         return containers
       }
@@ -48,7 +52,7 @@ export function useOutsideClick(
       }
 
       return [containers]
-    })()
+    })(containers)
 
     // Ignore if the target exists in one of the containers
     for (let container of _containers) {

diff --git a/packages/playground-react/package.json b/packages/playground-react/package.json
@@ -17,6 +17,7 @@
     "framer-motion": "^6.0.0",
     "next": "^12.0.8",
     "react": "16.14.0",
-    "react-dom": "16.14.0"
+    "react-dom": "16.14.0",
+    "react-flatpickr": "^3.10.9"
   }
 }
diff --git a/packages/playground-react/pages/dialog/dialog.tsx b/packages/playground-react/pages/dialog/dialog.tsx
@@ -1,8 +1,11 @@
 import React, { useState, Fragment } from 'react'
+import Flatpickr from 'react-flatpickr'
 import { Dialog, Menu, Portal, Transition } from '@headlessui/react'
 import { usePopper } from '../../utils/hooks/use-popper'
 import { classNames } from '../../utils/class-names'
 
+import 'flatpickr/dist/themes/light.css'
+
 function resolveClass({ active, disabled }) {
   return classNames(
     'flex justify-between w-full px-4 py-2 text-sm leading-5 text-left',
@@ -53,6 +56,8 @@ export default function Home() {
     modifiers: [{ name: 'offset', options: { offset: [0, 10] } }],
   })
 
+  let [date, setDate] = useState(new Date())
+
   return (
     <>
       <button
@@ -207,6 +212,7 @@ export default function Home() {
                               </Transition>
                             </Menu>
                           </div>
+                          <Flatpickr value={date} onChange={([date]) => setDate(date)} />
                         </div>
                       </div>
                     </div>

diff --git a/packages/playground-vue/package.json b/packages/playground-vue/package.json
@@ -16,6 +16,7 @@
   "dependencies": {
     "@headlessui/vue": "*",
     "vue": "^3.2.27",
+    "vue-flatpickr-component": "^9.0.5",
     "vue-router": "^4.0.0"
   },
   "devDependencies": {