Cancel outside click behavior on touch devices when scrolling (#3266)

* make `handleOutsideClick` stable

* cancel "outside click" when "scrolling" on touch device

When on a touch device, then the `touchend` event will fire, even if you
scrolled a bit and scrolling was your intention.

This now tracks that touches were at least 30px apart in either the X or
Y direction. If that's the case, then we do not consider it an outside
click.

* add `enabled` parameter to `useDocumentEvent` and `useWindowEvent`

* update `useDocumentEvent` and `useWindowEvent` usages

This now takes the new `enabled` value into account.

* update changelog

* bump vue and vite in playground

Author: RobinMalfait

package-lock.json

@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fix visual jitter in `Combobox` component when using native scrollbar ([#3190](https://github.com/tailwindlabs/headlessui/pull/3190))
 - Use `useId` instead of React internals (for React 19 compatibility) ([#3254](https://github.com/tailwindlabs/headlessui/pull/3254))
 - Ensure `ComboboxInput` does not sync with current value while typing ([#3259](https://github.com/tailwindlabs/headlessui/pull/3259))
+- Cancel outside click behavior on touch devices when scrolling ([#3266](https://github.com/tailwindlabs/headlessui/pull/3266))
 
 ## [2.0.4] - 2024-05-25
 

packages/@headlessui-react/CHANGELOG.md

@@ -2,18 +2,21 @@ import { useEffect } from 'react'
 import { useLatestValue } from './use-latest-value'
 
 export function useDocumentEvent<TType extends keyof DocumentEventMap>(
+  enabled: boolean,
   type: TType,
   listener: (ev: DocumentEventMap[TType]) => any,
   options?: boolean | AddEventListenerOptions
 ) {
   let listenerRef = useLatestValue(listener)
 
   useEffect(() => {
+    if (!enabled) return
+
     function handler(event: DocumentEventMap[TType]) {
       listenerRef.current(event)
     }
 
     document.addEventListener(type, handler, options)
     return () => document.removeEventListener(type, handler, options)
-  }, [type, options])
+  }, [enabled, type, options])
 }

packages/@headlessui-react/src/hooks/use-document-event.ts

@@ -1,132 +1,131 @@
-import { useEffect, useRef, type MutableRefObject } from 'react'
+import { useCallback, useRef, type MutableRefObject } from 'react'
 import { FocusableMode, isFocusableElement } from '../utils/focus-management'
 import { isMobile } from '../utils/platform'
 import { useDocumentEvent } from './use-document-event'
 import { useIsTopLayer } from './use-is-top-layer'
+import { useLatestValue } from './use-latest-value'
 import { useWindowEvent } from './use-window-event'
 
 type Container = MutableRefObject<HTMLElement | null> | HTMLElement | null
 type ContainerCollection = Container[] | Set<Container>
 type ContainerInput = Container | ContainerCollection
 
+// If the user moves their finger by ${MOVE_THRESHOLD_PX} pixels or more, we'll
+// assume that they are scrolling and not clicking. This will prevent the click
+// from being triggered when the user is scrolling.
+//
+// This also allows you to "cancel" the click by moving your finger more than
+// the threshold in pixels in any direction.
+const MOVE_THRESHOLD_PX = 30
+
 export function useOutsideClick(
   enabled: boolean,
   containers: ContainerInput | (() => ContainerInput),
   cb: (event: MouseEvent | PointerEvent | FocusEvent | TouchEvent, target: HTMLElement) => void
 ) {
   let isTopLayer = useIsTopLayer(enabled, 'outside-click')
+  let cbRef = useLatestValue(cb)
 
-  // TODO: remove this once the React bug has been fixed: https://github.com/facebook/react/issues/24657
-  let enabledRef = useRef(false)
-  useEffect(
-    process.env.NODE_ENV === 'test'
-      ? () => {
-          enabledRef.current = isTopLayer
-        }
-      : () => {
-          requestAnimationFrame(() => {
-            enabledRef.current = isTopLayer
-          })
-        },
-    [isTopLayer]
-  )
-
-  function handleOutsideClick<E extends MouseEvent | PointerEvent | FocusEvent | TouchEvent>(
-    event: E,
-    resolveTarget: (event: E) => HTMLElement | null
-  ) {
-    if (!enabledRef.current) return
+  let handleOutsideClick = useCallback(
+    function handleOutsideClick<E extends MouseEvent | PointerEvent | FocusEvent | TouchEvent>(
+      event: E,
+      resolveTarget: (event: E) => HTMLElement | null
+    ) {
+      // Check whether the event got prevented already. This can happen if you
+      // use the useOutsideClick hook in both a Dialog and a Menu and the inner
+      // Menu "cancels" the default behavior so that only the Menu closes and
+      // not the Dialog (yet)
+      if (event.defaultPrevented) return
 
-    // Check whether the event got prevented already. This can happen if you use the
-    // useOutsideClick hook in both a Dialog and a Menu and the inner Menu "cancels" the default
-    // behavior so that only the Menu closes and not the Dialog (yet)
-    if (event.defaultPrevented) return
+      let target = resolveTarget(event)
 
-    let target = resolveTarget(event)
+      if (target === null) {
+        return
+      }
 
-    if (target === null) {
-      return
-    }
+      // Ignore if the target doesn't exist in the DOM anymore
+      if (!target.getRootNode().contains(target)) return
 
-    // Ignore if the target doesn't exist in the DOM anymore
-    if (!target.getRootNode().contains(target)) return
+      // Ignore if the target was removed from the DOM by the time the handler
+      // was called
+      if (!target.isConnected) return
 
-    // Ignore if the target was removed from the DOM by the time the handler was called
-    if (!target.isConnected) return
+      let _containers = (function resolve(containers): ContainerCollection {
+        if (typeof containers === 'function') {
+          return resolve(containers())
+        }
 
-    let _containers = (function resolve(containers): ContainerCollection {
-      if (typeof containers === 'function') {
-        return resolve(containers())
-      }
+        if (Array.isArray(containers)) {
+          return containers
+        }
 
-      if (Array.isArray(containers)) {
-        return containers
-      }
+        if (containers instanceof Set) {
+          return containers
+        }
 
-      if (containers instanceof Set) {
-        return containers
-      }
+        return [containers]
+      })(containers)
 
-      return [containers]
-    })(containers)
+      // Ignore if the target exists in one of the containers
+      for (let container of _containers) {
+        if (container === null) continue
+        let domNode = container instanceof HTMLElement ? container : container.current
+        if (domNode?.contains(target)) {
+          return
+        }
 
-    // Ignore if the target exists in one of the containers
-    for (let container of _containers) {
-      if (container === null) continue
-      let domNode = container instanceof HTMLElement ? container : container.current
-      if (domNode?.contains(target)) {
-        return
+        // If the click crossed a shadow boundary, we need to check if the
+        // container is inside the tree by using `composedPath` to "pierce" the
+        // shadow boundary
+        if (event.composed && event.composedPath().includes(domNode as EventTarget)) {
+          return
+        }
       }
 
-      // If the click crossed a shadow boundary, we need to check if the container
-      // is inside the tree by using `composedPath` to "pierce" the shadow boundary
-      if (event.composed && event.composedPath().includes(domNode as EventTarget)) {
-        return
+      // This allows us to check whether the event was defaultPrevented when you
+      // are nesting this inside a `<Dialog />` for example.
+      if (
+        // This check allows us to know whether or not we clicked on a
+        // "focusable" element like a button or an input. This is a backwards
+        // compatibility check so that you can open a <Menu /> and click on
+        // another <Menu /> which should close Menu A and open Menu B. We might
+        // revisit that so that you will require 2 clicks instead.
+        !isFocusableElement(target, FocusableMode.Loose) &&
+        // This could be improved, but the `Combobox.Button` adds tabIndex={-1}
+        // to make it unfocusable via the keyboard so that tabbing to the next
+        // item from the input doesn't first go to the button.
+        target.tabIndex !== -1
+      ) {
+        event.preventDefault()
       }
-    }
-
-    // This allows us to check whether the event was defaultPrevented when you are nesting this
-    // inside a `<Dialog />` for example.
-    if (
-      // This check allows us to know whether or not we clicked on a "focusable" element like a
-      // button or an input. This is a backwards compatibility check so that you can open a <Menu
-      // /> and click on another <Menu /> which should close Menu A and open Menu B. We might
-      // revisit that so that you will require 2 clicks instead.
-      !isFocusableElement(target, FocusableMode.Loose) &&
-      // This could be improved, but the `Combobox.Button` adds tabIndex={-1} to make it
-      // unfocusable via the keyboard so that tabbing to the next item from the input doesn't
-      // first go to the button.
-      target.tabIndex !== -1
-    ) {
-      event.preventDefault()
-    }
 
-    return cb(event, target)
-  }
+      return cbRef.current(event, target)
+    },
+    [cbRef]
+  )
 
   let initialClickTarget = useRef<EventTarget | null>(null)
 
   useDocumentEvent(
+    isTopLayer,
     'pointerdown',
     (event) => {
-      if (enabledRef.current) {
-        initialClickTarget.current = event.composedPath?.()?.[0] || event.target
-      }
+      initialClickTarget.current = event.composedPath?.()?.[0] || event.target
     },
     true
   )
 
   useDocumentEvent(
+    isTopLayer,
     'mousedown',
     (event) => {
-      if (enabledRef.current) {
-        initialClickTarget.current = event.composedPath?.()?.[0] || event.target
-      }
+      initialClickTarget.current = event.composedPath?.()?.[0] || event.target
     },
     true
   )
 
   useDocumentEvent(
+    isTopLayer,
     'click',
     (event) => {
       if (isMobile()) {
@@ -151,9 +150,31 @@ export function useOutsideClick(
     true
   )
 
+  let startPosition = useRef({ x: 0, y: 0 })
   useDocumentEvent(
+    isTopLayer,
+    'touchstart',
+    (event) => {
+      startPosition.current.x = event.touches[0].clientX
+      startPosition.current.y = event.touches[0].clientY
+    },
+    true
+  )
+
+  useDocumentEvent(
+    isTopLayer,
     'touchend',
     (event) => {
+      // If the user moves their finger by ${MOVE_THRESHOLD_PX} pixels or more,
+      // we'll assume that they are scrolling and not clicking.
+      let endPosition = { x: event.changedTouches[0].clientX, y: event.changedTouches[0].clientY }
+      if (
+        Math.abs(endPosition.x - startPosition.current.x) >= MOVE_THRESHOLD_PX ||
+        Math.abs(endPosition.y - startPosition.current.y) >= MOVE_THRESHOLD_PX
+      ) {
+        return
+      }
+
       return handleOutsideClick(event, () => {
         if (event.target instanceof HTMLElement) {
           return event.target
@@ -177,6 +198,7 @@ export function useOutsideClick(
   // If so this was because of a click, focus, or other interaction with the child iframe
   // and we can consider it an "outside click"
   useWindowEvent(
+    isTopLayer,
     'blur',
     (event) => {
       return handleOutsideClick(event, () => {

packages/@headlessui-react/src/hooks/use-outside-click.ts

@@ -8,8 +8,10 @@ export enum Direction {
 
 export function useTabDirection() {
   let direction = useRef(Direction.Forwards)
+  let enabled = true
 
   useWindowEvent(
+    enabled,
     'keydown',
     (event) => {
       if (event.key === 'Tab') {

packages/@headlessui-react/src/hooks/use-tab-direction.ts

@@ -2,18 +2,21 @@ import { useEffect } from 'react'
 import { useLatestValue } from './use-latest-value'
 
 export function useWindowEvent<TType extends keyof WindowEventMap>(
+  enabled: boolean,
   type: TType,
   listener: (ev: WindowEventMap[TType]) => any,
   options?: boolean | AddEventListenerOptions
 ) {
   let listenerRef = useLatestValue(listener)
 
   useEffect(() => {
+    if (!enabled) return
+
     function handler(event: WindowEventMap[TType]) {
       listenerRef.current(event)
     }
 
     window.addEventListener(type, handler, options)
     return () => window.removeEventListener(type, handler, options)
-  }, [type, options])
+  }, [enabled, type, options])
 }

packages/@headlessui-react/src/hooks/use-window-event.ts

@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add `immediate` prop to `<Combobox />` for immediately opening the Combobox when the `input` receives focus ([#2686](https://github.com/tailwindlabs/headlessui/pull/2686))
 - Add `virtual` prop to `Combobox` component ([#2779](https://github.com/tailwindlabs/headlessui/pull/2779))
 
+### Fixed
+
+- Cancel outside click behavior on touch devices when scrolling ([#3266](https://github.com/tailwindlabs/headlessui/pull/3266))
+
 ## [1.7.22] - 2024-05-08
 
 ### Fixed

packages/@headlessui-vue/CHANGELOG.md

@@ -1,14 +1,17 @@
-import { watchEffect } from 'vue'
+import { watchEffect, type Ref } from 'vue'
 import { env } from '../utils/env'
 
 export function useDocumentEvent<TType extends keyof DocumentEventMap>(
+  enabled: Ref<boolean>,
   type: TType,
   listener: (this: Document, ev: DocumentEventMap[TType]) => any,
   options?: boolean | AddEventListenerOptions
 ) {
   if (env.isServer) return
 
   watchEffect((onInvalidate) => {
+    if (!enabled.value) return
+
     document.addEventListener(type, listener, options)
     onInvalidate(() => document.removeEventListener(type, listener, options))
   })