From f097a8d24dbea944876fe6b4fd02e42b496cf862 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Tue, 12 Jul 2022 12:24:32 -0400 Subject: [PATCH] Adjust outside click handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Don’t close dialog if opened during mouse up event - Don’t close dialog if drag starts inside dialog and ends outside dialog --- .../src/components/dialog/dialog.test.tsx | 77 ++++++++++++++- .../src/hooks/use-outside-click.ts | 24 ++++- .../src/test-utils/interactions.ts | 68 +++++++++++++ .../src/components/dialog/dialog.test.ts | 95 ++++++++++++++++++- .../src/hooks/use-outside-click.ts | 26 ++++- .../src/test-utils/interactions.ts | 68 +++++++++++++ 6 files changed, 353 insertions(+), 5 deletions(-) diff --git a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx index 21971b8409..f660254c31 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx @@ -17,7 +17,7 @@ import { getDialogs, getDialogOverlays, } from '../../test-utils/accessibility-assertions' -import { click, press, Keys } from '../../test-utils/interactions' +import { click, mouseDrag, press, Keys } from '../../test-utils/interactions' import { PropsOf } from '../../types' import { Transition } from '../transitions/transition' import { createPortal } from 'react-dom' @@ -1066,6 +1066,81 @@ describe('Mouse interactions', () => { assertDialog({ state: DialogState.Visible }) }) ) + + it( + 'should not close the dialog if opened during mouse up', + suppressConsoleLogs(async () => { + function Example() { + let [isOpen, setIsOpen] = useState(false) + return ( + <> + + + + + + + + + + ) + } + + render() + + await click(document.getElementById('trigger')) + + assertDialog({ state: DialogState.Visible }) + + await click(document.getElementById('inside')) + + assertDialog({ state: DialogState.Visible }) + }) + ) + + it( + 'should not close the dialog if click starts inside the dialog but ends outside', + suppressConsoleLogs(async () => { + function Example() { + let [isOpen, setIsOpen] = useState(false) + return ( + <> + +
this thing
+ + + + + + + + + ) + } + + render() + + // Open the dialog + await click(document.getElementById('trigger')) + + assertDialog({ state: DialogState.Visible }) + + // Start a click inside the dialog and end it outside + await mouseDrag(document.getElementById('inside'), document.getElementById('imoutside')) + + // It should not have hidden + assertDialog({ state: DialogState.Visible }) + + await click(document.getElementById('imoutside')) + + // It's gone + assertDialog({ state: DialogState.InvisibleUnmounted }) + }) + ) }) describe('Nesting', () => { diff --git a/packages/@headlessui-react/src/hooks/use-outside-click.ts b/packages/@headlessui-react/src/hooks/use-outside-click.ts index d358023afb..30c5e8d9f1 100644 --- a/packages/@headlessui-react/src/hooks/use-outside-click.ts +++ b/packages/@headlessui-react/src/hooks/use-outside-click.ts @@ -90,9 +90,31 @@ export function useOutsideClick( return cb(event, target) } + let initialClickTarget = useRef(null) + + useWindowEvent( + 'mousedown', + (event) => { + if (enabledRef.current) { + initialClickTarget.current = event.target + } + }, + true + ) + useWindowEvent( 'click', - (event) => handleOutsideClick(event, (event) => event.target as HTMLElement), + (event) => { + if (!initialClickTarget.current) { + return + } + + handleOutsideClick(event, () => { + return initialClickTarget.current as HTMLElement + }) + + initialClickTarget.current = null + }, // We will use the `capture` phase so that layers in between with `event.stopPropagation()` // don't "cancel" this outside click check. E.g.: A `Menu` inside a `DialogPanel` if the `Menu` diff --git a/packages/@headlessui-react/src/test-utils/interactions.ts b/packages/@headlessui-react/src/test-utils/interactions.ts index 2b7976dae7..2a71b96560 100644 --- a/packages/@headlessui-react/src/test-utils/interactions.ts +++ b/packages/@headlessui-react/src/test-utils/interactions.ts @@ -344,6 +344,74 @@ export async function mouseLeave(element: Document | Element | Window | null) { } } +export async function mouseDrag( + startingElement: Document | Element | Window | Node | null, + endingElement: Document | Element | Window | Node | null +) { + let button = MouseButton.Left + + try { + if (startingElement === null) return expect(startingElement).not.toBe(null) + if (endingElement === null) return expect(endingElement).not.toBe(null) + if (startingElement instanceof HTMLButtonElement && startingElement.disabled) return + + let options = { button } + + // Cancel in pointerDown cancels mouseDown, mouseUp + let cancelled = !fireEvent.pointerDown(startingElement, options) + + if (!cancelled) { + cancelled = !fireEvent.mouseDown(startingElement, options) + } + + // Ensure to trigger a `focus` event if the element is focusable, or within a focusable element + if (!cancelled) { + let next: HTMLElement | null = startingElement as HTMLElement | null + while (next !== null) { + if (next.matches(focusableSelector)) { + next.focus() + break + } + next = next.parentElement + } + } + + fireEvent.pointerMove(startingElement, options) + if (!cancelled) { + fireEvent.mouseMove(startingElement, options) + } + + fireEvent.pointerOut(startingElement, options) + if (!cancelled) { + fireEvent.mouseOut(startingElement, options) + } + + // crosses over to the ending element + + fireEvent.pointerOver(endingElement, options) + if (!cancelled) { + fireEvent.mouseOver(endingElement, options) + } + + fireEvent.pointerMove(endingElement, options) + if (!cancelled) { + fireEvent.mouseMove(endingElement, options) + } + + fireEvent.pointerUp(endingElement, options) + if (!cancelled) { + fireEvent.mouseUp(endingElement, options) + } + + fireEvent.click(endingElement, options) + + await new Promise(nextFrame) + } catch (err) { + if (err instanceof Error) Error.captureStackTrace(err, click) + throw err + } +} + // --- function focusNext(event: Partial) { diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts index c5d9b3776a..1d5266626c 100644 --- a/packages/@headlessui-vue/src/components/dialog/dialog.test.ts +++ b/packages/@headlessui-vue/src/components/dialog/dialog.test.ts @@ -25,7 +25,7 @@ import { getDialogs, getDialogOverlays, } from '../../test-utils/accessibility-assertions' -import { click, press, Keys } from '../../test-utils/interactions' +import { click, mouseDrag, press, Keys } from '../../test-utils/interactions' import { html } from '../../test-utils/html' // @ts-expect-error @@ -1444,6 +1444,99 @@ describe('Mouse interactions', () => { assertDialog({ state: DialogState.Visible }) }) ) + + fit( + 'should not close the dialog if opened during mouse up', + suppressConsoleLogs(async () => { + renderTemplate({ + template: ` +
+ + + + + + + + +
+ `, + setup() { + let isOpen = ref(false) + return { + isOpen, + setIsOpen(value: boolean) { + isOpen.value = value + }, + toggleOpen() { + isOpen.value = !isOpen.value + }, + } + }, + }) + + await click(document.getElementById('trigger')) + + assertDialog({ state: DialogState.Visible }) + + await click(document.getElementById('inside')) + + assertDialog({ state: DialogState.Visible }) + }) + ) + + it( + 'should not close the dialog if click starts inside the dialog but ends outside', + suppressConsoleLogs(async () => { + renderTemplate({ + template: ` +
+ +
this thing
+ + + + + + + +
+ `, + setup() { + let isOpen = ref(false) + return { + isOpen, + setIsOpen(value: boolean) { + isOpen.value = value + }, + toggleOpen() { + isOpen.value = !isOpen.value + }, + } + }, + }) + + // Open the dialog + await click(document.getElementById('trigger')) + + assertDialog({ state: DialogState.Visible }) + + // Start a click inside the dialog and end it outside + await mouseDrag(document.getElementById('inside'), document.getElementById('imoutside')) + + // It should not have hidden + assertDialog({ state: DialogState.Visible }) + + await click(document.getElementById('imoutside')) + + // It's gone + assertDialog({ state: DialogState.InvisibleUnmounted }) + }) + ) }) describe('Nesting', () => { diff --git a/packages/@headlessui-vue/src/hooks/use-outside-click.ts b/packages/@headlessui-vue/src/hooks/use-outside-click.ts index 83d8e652b5..a0c412673c 100644 --- a/packages/@headlessui-vue/src/hooks/use-outside-click.ts +++ b/packages/@headlessui-vue/src/hooks/use-outside-click.ts @@ -1,5 +1,5 @@ import { useWindowEvent } from './use-window-event' -import { computed, Ref, ComputedRef } from 'vue' +import { computed, Ref, ComputedRef, ref } from 'vue' import { FocusableMode, isFocusableElement } from '../utils/focus-management' import { dom } from '../utils/dom' @@ -76,9 +76,31 @@ export function useOutsideClick( return cb(event, target) } + let initialClickTarget = ref(null) + + useWindowEvent( + 'mousedown', + (event) => { + if (enabled.value) { + initialClickTarget.value = event.target + } + }, + true + ) + useWindowEvent( 'click', - (event) => handleOutsideClick(event, (event) => event.target as HTMLElement), + (event) => { + if (!initialClickTarget.value) { + return + } + + handleOutsideClick(event, (event) => { + return event.target as HTMLElement + }) + + initialClickTarget.value = null + }, // We will use the `capture` phase so that layers in between with `event.stopPropagation()` // don't "cancel" this outside click check. E.g.: A `Menu` inside a `DialogPanel` if the `Menu` diff --git a/packages/@headlessui-vue/src/test-utils/interactions.ts b/packages/@headlessui-vue/src/test-utils/interactions.ts index 4f99cb731d..01c7a3d7a1 100644 --- a/packages/@headlessui-vue/src/test-utils/interactions.ts +++ b/packages/@headlessui-vue/src/test-utils/interactions.ts @@ -338,6 +338,74 @@ export async function mouseLeave(element: Document | Element | Window | null) { } } +export async function mouseDrag( + startingElement: Document | Element | Window | Node | null, + endingElement: Document | Element | Window | Node | null +) { + let button = MouseButton.Left + + try { + if (startingElement === null) return expect(startingElement).not.toBe(null) + if (endingElement === null) return expect(endingElement).not.toBe(null) + if (startingElement instanceof HTMLButtonElement && startingElement.disabled) return + + let options = { button } + + // Cancel in pointerDown cancels mouseDown, mouseUp + let cancelled = !fireEvent.pointerDown(startingElement, options) + + if (!cancelled) { + cancelled = !fireEvent.mouseDown(startingElement, options) + } + + // Ensure to trigger a `focus` event if the element is focusable, or within a focusable element + if (!cancelled) { + let next: HTMLElement | null = startingElement as HTMLElement | null + while (next !== null) { + if (next.matches(focusableSelector)) { + next.focus() + break + } + next = next.parentElement + } + } + + fireEvent.pointerMove(startingElement, options) + if (!cancelled) { + fireEvent.mouseMove(startingElement, options) + } + + fireEvent.pointerOut(startingElement, options) + if (!cancelled) { + fireEvent.mouseOut(startingElement, options) + } + + // crosses over to the ending element + + fireEvent.pointerOver(endingElement, options) + if (!cancelled) { + fireEvent.mouseOver(endingElement, options) + } + + fireEvent.pointerMove(endingElement, options) + if (!cancelled) { + fireEvent.mouseMove(endingElement, options) + } + + fireEvent.pointerUp(endingElement, options) + if (!cancelled) { + fireEvent.mouseUp(endingElement, options) + } + + fireEvent.click(endingElement, options) + + await new Promise(nextFrame) + } catch (err) { + if (err instanceof Error) Error.captureStackTrace(err, click) + throw err + } +} + // --- function focusNext(event: Partial) {