Improve outside click of Dialog component

packages/@headlessui-react/CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fix incorrect transitionend/transitioncancel events for the Transition component ([#1537](https://github.com/tailwindlabs/headlessui/pull/1537))
+- Improve outside click of `Dialog` component ([#1546](https://github.com/tailwindlabs/headlessui/pull/1546))
 
 ## [1.6.4] - 2022-05-29
 

packages/@headlessui-react/src/components/combobox/combobox.tsx

@@ -34,7 +34,7 @@ import { forwardRefWithAs, render, compact, PropsForFeatures, Features } from '.
 import { isDisabledReactIssue7711 } from '../../utils/bugs'
 import { match } from '../../utils/match'
 import { objectToFormEntries } from '../../utils/form'
-import { sortByDomNode } from '../../utils/focus-management'
+import { FocusableMode, isFocusableElement, sortByDomNode } from '../../utils/focus-management'
 
 import { Hidden, Features as HiddenFeatures } from '../../internal/hidden'
 import { useOpenClosed, State, OpenClosedProvider } from '../../internal/open-closed'
@@ -415,11 +415,11 @@ let ComboboxRoot = forwardRefWithAs(function Combobox<
   }, [data])
 
   // Handle outside click
-  useOutsideClick([data.buttonRef, data.inputRef, data.optionsRef], () => {
-    if (data.comboboxState !== ComboboxState.Open) return
-
-    dispatch({ type: ActionTypes.CloseCombobox })
-  })
+  useOutsideClick(
+    [data.buttonRef, data.inputRef, data.optionsRef],
+    () => dispatch({ type: ActionTypes.CloseCombobox }),
+    data.comboboxState === ComboboxState.Open
+  )
 
   let slot = useMemo<ComboboxRenderPropArg<TType>>(
     () => ({

packages/@headlessui-react/src/components/dialog/dialog.test.tsx

@@ -889,9 +889,11 @@ describe('Mouse interactions', () => {
         return (
           <div onClick={wrapperFn}>
             <Dialog open={isOpen} onClose={setIsOpen}>
-              Contents
-              <button onClick={() => setIsOpen(false)}>Inside</button>
-              <TabSentinel />
+              <Dialog.Panel>
+                Contents
+                <button onClick={() => setIsOpen(false)}>Inside</button>
+                <TabSentinel />
+              </Dialog.Panel>
             </Dialog>
           </div>
         )

packages/@headlessui-react/src/components/dialog/dialog.tsx

@@ -32,7 +32,7 @@ import { Description, useDescriptions } from '../description/description'
 import { useOpenClosed, State } from '../../internal/open-closed'
 import { useServerHandoffComplete } from '../../hooks/use-server-handoff-complete'
 import { StackProvider, StackMessage } from '../../internal/stack-context'
-import { useOutsideClick, Features as OutsideClickFeatures } from '../../hooks/use-outside-click'
+import { useOutsideClick } from '../../hooks/use-outside-click'
 import { getOwnerDocument } from '../../utils/owner'
 import { useOwnerDocument } from '../../hooks/use-owner'
 import { useEventListener } from '../../hooks/use-event-listener'
@@ -100,13 +100,7 @@ let DEFAULT_DIALOG_TAG = 'div' as const
 interface DialogRenderPropArg {
   open: boolean
 }
-type DialogPropsWeControl =
-  | 'id'
-  | 'role'
-  | 'aria-modal'
-  | 'aria-describedby'
-  | 'aria-labelledby'
-  | 'onClick'
+type DialogPropsWeControl = 'id' | 'role' | 'aria-modal' | 'aria-describedby' | 'aria-labelledby'
 
 let DialogRenderFeatures = Features.RenderStrategy | Features.Static
 
@@ -204,27 +198,22 @@ let DialogRoot = forwardRefWithAs(function Dialog<
   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(mainTreeNode.current)) return false // Skip if it is the main app
-          if (state.panelRef.current && container.contains(state.panelRef.current)) return false
-          return true // Keep
-        }
-      )
+      let rootContainers = Array.from(
+        ownerDocument?.querySelectorAll('body > *, [data-headlessui-portal]') ?? []
+      ).filter((container) => {
+        if (!(container instanceof HTMLElement)) return false // Skip non-HTMLElements
+        if (container.contains(mainTreeNode.current)) return false // Skip if it is the main app
+        if (state.panelRef.current && container.contains(state.panelRef.current)) return false
+        return true // Keep
+      })
 
       return [
         ...rootContainers,
         state.panelRef.current ?? internalDialogRef.current,
       ] as HTMLElement[]
     },
-    () => {
-      if (dialogState !== DialogStates.Open) return
-      if (hasNestedDialogs) return
-
-      close()
-    },
-    OutsideClickFeatures.IgnoreScrollbars
+    close,
+    enabled && !hasNestedDialogs
   )
 
   // Handle `Escape` to close
@@ -311,9 +300,6 @@ let DialogRoot = forwardRefWithAs(function Dialog<
     'aria-modal': dialogState === DialogStates.Open ? true : undefined,
     'aria-labelledby': state.titleId,
     'aria-describedby': describedby,
-    onClick(event: ReactMouseEvent) {
-      event.stopPropagation()
-    },
   }
 
   return (
@@ -492,10 +478,17 @@ let Panel = forwardRefWithAs(function Panel<TTag extends ElementType = typeof DE
     [dialogState]
   )
 
+  // Prevent the click events inside the Dialog.Panel from bubbling through the React Tree which
+  // could submit wrapping <form> elements even if we portalled the Dialog.
+  let handleClick = useEvent((event: ReactMouseEvent) => {
+    event.stopPropagation()
+  })
+
   let theirProps = props
   let ourProps = {
     ref: panelRef,
     id,
+    onClick: handleClick,
   }
 
   return render({

packages/@headlessui-react/src/components/focus-trap/focus-trap.tsx

@@ -147,7 +147,10 @@ function useRestoreFocus({ ownerDocument }: { ownerDocument: Document | null },
   useWatch(() => {
     if (enabled) return
 
-    focusElement(restoreElement.current)
+    if (ownerDocument?.activeElement === ownerDocument?.body) {
+      focusElement(restoreElement.current)
+    }
+
     restoreElement.current = null
   }, [enabled])
 

packages/@headlessui-react/src/components/listbox/listbox.tsx

@@ -396,16 +396,18 @@ let ListboxRoot = forwardRefWithAs(function Listbox<
   )
 
   // Handle outside click
-  useOutsideClick([buttonRef, optionsRef], (event, target) => {
-    if (listboxState !== ListboxStates.Open) return
-
-    dispatch({ type: ActionTypes.CloseListbox })
+  useOutsideClick(
+    [buttonRef, optionsRef],
+    (event, target) => {
+      dispatch({ type: ActionTypes.CloseListbox })
 
-    if (!isFocusableElement(target, FocusableMode.Loose)) {
-      event.preventDefault()
-      buttonRef.current?.focus()
-    }
-  })
+      if (!isFocusableElement(target, FocusableMode.Loose)) {
+        event.preventDefault()
+        buttonRef.current?.focus()
+      }
+    },
+    listboxState === ListboxStates.Open
+  )
 
   let slot = useMemo<ListboxRenderPropArg>(
     () => ({ open: listboxState === ListboxStates.Open, disabled }),

packages/@headlessui-react/src/components/menu/menu.tsx

@@ -239,16 +239,18 @@ let MenuRoot = forwardRefWithAs(function Menu<TTag extends ElementType = typeof
   let menuRef = useSyncRefs(ref)
 
   // Handle outside click
-  useOutsideClick([buttonRef, itemsRef], (event, target) => {
-    if (menuState !== MenuStates.Open) return
-
-    dispatch({ type: ActionTypes.CloseMenu })
+  useOutsideClick(
+    [buttonRef, itemsRef],
+    (event, target) => {
+      dispatch({ type: ActionTypes.CloseMenu })
 
-    if (!isFocusableElement(target, FocusableMode.Loose)) {
-      event.preventDefault()
-      buttonRef.current?.focus()
-    }
-  })
+      if (!isFocusableElement(target, FocusableMode.Loose)) {
+        event.preventDefault()
+        buttonRef.current?.focus()
+      }
+    },
+    menuState === MenuStates.Open
+  )
 
   let slot = useMemo<MenuRenderPropArg>(
     () => ({ open: menuState === MenuStates.Open }),
@@ -344,7 +346,6 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
       d.nextFrame(() => state.buttonRef.current?.focus({ preventScroll: true }))
     } else {
       event.preventDefault()
-      event.stopPropagation()
       dispatch({ type: ActionTypes.OpenMenu })
     }
   })

packages/@headlessui-react/src/components/popover/popover.tsx

@@ -258,16 +258,18 @@ let PopoverRoot = forwardRefWithAs(function Popover<
   )
 
   // Handle outside click
-  useOutsideClick([button, panel], (event, target) => {
-    if (popoverState !== PopoverStates.Open) return
-
-    dispatch({ type: ActionTypes.ClosePopover })
+  useOutsideClick(
+    [button, panel],
+    (event, target) => {
+      dispatch({ type: ActionTypes.ClosePopover })
 
-    if (!isFocusableElement(target, FocusableMode.Loose)) {
-      event.preventDefault()
-      button?.focus()
-    }
-  })
+      if (!isFocusableElement(target, FocusableMode.Loose)) {
+        event.preventDefault()
+        button?.focus()
+      }
+    },
+    popoverState === PopoverStates.Open
+  )
 
   let close = useEvent((focusableElement?: HTMLElement | MutableRefObject<HTMLElement | null>) => {
     dispatch({ type: ActionTypes.ClosePopover })

packages/@headlessui-react/src/components/portal/portal.test.tsx

@@ -299,6 +299,6 @@ it('should be possible to force the Portal into a specific element using Portal.
   render(<Example />)
 
   expect(document.body.innerHTML).toMatchInlineSnapshot(
-    `"<div><main><aside id=\\"group-1\\">A<div>Next to A</div></aside><section id=\\"group-2\\"><span>B</span></section></main></div><div id=\\"headlessui-portal-root\\"><div>I am in the portal root</div></div>"`
+    `"<div><main><aside id=\\"group-1\\">A<div data-headlessui-portal=\\"\\">Next to A</div></aside><section id=\\"group-2\\"><span>B</span></section></main></div><div id=\\"headlessui-portal-root\\"><div data-headlessui-portal=\\"\\">I am in the portal root</div></div>"`
   )
 })

packages/@headlessui-react/src/components/portal/portal.tsx

@@ -95,6 +95,7 @@ let PortalRoot = forwardRefWithAs(function Portal<
     // Element already exists in target, always calling target.appendChild(element) will cause a
     // brief unmount/remount.
     if (!target.contains(element)) {
+      element.setAttribute('data-headlessui-portal', '')
       target.appendChild(element)
     }