Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel outside click behavior on touch devices when scrolling #3266

Merged
merged 6 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,510 changes: 1,239 additions & 271 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion packages/@headlessui-react/src/hooks/use-document-event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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])
}
182 changes: 102 additions & 80 deletions packages/@headlessui-react/src/hooks/use-outside-click.ts
Original file line number Diff line number Diff line change
@@ -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]
)

Comment on lines -21 to -32
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked issue was expected behavior, since then we've also restructured some of this logic and this doesn't happen anymore.

The issue we were seeing was that opening a <Dialog /> by setting state in an onClick, registered a click listener on the window and immediately triggered it so the <Dialog /> was immediately closed.

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()) {
Expand All @@ -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
Expand All @@ -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, () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/@headlessui-react/src/hooks/use-tab-direction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
5 changes: 4 additions & 1 deletion packages/@headlessui-react/src/hooks/use-window-event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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])
}
4 changes: 4 additions & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion packages/@headlessui-vue/src/hooks/use-document-event.ts
Original file line number Diff line number Diff line change
@@ -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))
})
Expand Down
Loading
Loading