From a4e2ce6fe34347deb251b87b00346e72c1bada09 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 2 Jun 2022 14:47:04 +0200 Subject: [PATCH] WIP --- .../src/components/dialog/dialog.test.tsx | 10 ++- .../src/components/dialog/dialog.tsx | 22 +++-- .../src/hooks/use-outside-click.ts | 48 +++------- .../pages/combinations/tabs-in-dialog.tsx | 7 +- .../pages/dialog/scrollable-dialog.tsx | 90 +++++++++++++++++++ 5 files changed, 121 insertions(+), 56 deletions(-) create mode 100644 packages/playground-react/pages/dialog/scrollable-dialog.tsx diff --git a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx index c59e3041c0..b3152a0178 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.test.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.test.tsx @@ -781,7 +781,7 @@ describe('Mouse interactions', () => { }) ) - it( + fit( 'should be possible to close the dialog, and keep focus on the focusable element', suppressConsoleLogs(async () => { function Example() { @@ -889,9 +889,11 @@ describe('Mouse interactions', () => { return (
- Contents - - + + Contents + + +
) diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index 8d4b42cec8..859fda748f 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/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 @@ -224,7 +218,7 @@ let DialogRoot = forwardRefWithAs(function Dialog< close() }, - OutsideClickFeatures.IgnoreScrollbars + enabled ) // Handle `Escape` to close @@ -311,9 +305,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 +483,17 @@ let Panel = forwardRefWithAs(function Panel 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({ diff --git a/packages/@headlessui-react/src/hooks/use-outside-click.ts b/packages/@headlessui-react/src/hooks/use-outside-click.ts index 8cef785e08..7c1b4a9607 100644 --- a/packages/@headlessui-react/src/hooks/use-outside-click.ts +++ b/packages/@headlessui-react/src/hooks/use-outside-click.ts @@ -1,29 +1,25 @@ -import { MutableRefObject, useRef } from 'react' -import { microTask } from '../utils/micro-task' -import { useEvent } from './use-event' +import { MutableRefObject, useEffect, useRef } from 'react' import { useWindowEvent } from './use-window-event' type Container = MutableRefObject | HTMLElement | null type ContainerCollection = Container[] | Set type ContainerInput = Container | ContainerCollection -export enum Features { - None = 1 << 0, - IgnoreScrollbars = 1 << 1, -} - export function useOutsideClick( containers: ContainerInput | (() => ContainerInput), cb: (event: MouseEvent | PointerEvent, target: HTMLElement) => void, - features: Features = Features.None + enabled: boolean = true ) { - let called = useRef(false) - let handler = useEvent((event: MouseEvent | PointerEvent) => { - if (called.current) return - called.current = true - microTask(() => { - called.current = false + // TODO: remove this once the React bug has been fixed: https://github.com/facebook/react/issues/24657 + let enabledRef = useRef(false) + useEffect(() => { + requestAnimationFrame(() => { + enabledRef.current = enabled }) + }, [enabled]) + + useWindowEvent('click', (event) => { + if (!enabledRef.current) return let _containers = (function resolve(containers): ContainerCollection { if (typeof containers === 'function') { @@ -46,25 +42,6 @@ export function useOutsideClick( // Ignore if the target doesn't exist in the DOM anymore if (!target.ownerDocument.documentElement.contains(target)) return - // Ignore scrollbars: - // This is a bit hacky, and is only necessary because we are checking for `pointerdown` and - // `mousedown` events. They _are_ being called if you click on a scrollbar. The `click` event - // is not called when clicking on a scrollbar, but we can't use that otherwise it won't work - // on mobile devices where only pointer events are being used. - if ((features & Features.IgnoreScrollbars) === Features.IgnoreScrollbars) { - // TODO: We can calculate this dynamically~is. On macOS if you have the "Automatically based - // on mouse or trackpad" setting enabled, then the scrollbar will float on top and therefore - // you can't calculate its with by checking the clientWidth and scrollWidth of the element. - // Therefore we are currently hardcoding this to be 20px. - let scrollbarWidth = 20 - - let viewport = target.ownerDocument.documentElement - if (event.clientX > viewport.clientWidth - scrollbarWidth) return - if (event.clientX < scrollbarWidth) return - if (event.clientY > viewport.clientHeight - scrollbarWidth) return - if (event.clientY < scrollbarWidth) return - } - // Ignore if the target exists in one of the containers for (let container of _containers) { if (container === null) continue @@ -76,7 +53,4 @@ export function useOutsideClick( return cb(event, target) }) - - useWindowEvent('pointerdown', handler) - useWindowEvent('mousedown', handler) } diff --git a/packages/playground-react/pages/combinations/tabs-in-dialog.tsx b/packages/playground-react/pages/combinations/tabs-in-dialog.tsx index bca77d568c..f879fb433f 100644 --- a/packages/playground-react/pages/combinations/tabs-in-dialog.tsx +++ b/packages/playground-react/pages/combinations/tabs-in-dialog.tsx @@ -3,13 +3,14 @@ import { Dialog, Tab } from '@headlessui/react' export default function App() { let [open, setOpen] = useState(false) + console.log({ open }) return ( <> - -
+
+
@@ -24,7 +25,7 @@ export default function App() {
-
+
) diff --git a/packages/playground-react/pages/dialog/scrollable-dialog.tsx b/packages/playground-react/pages/dialog/scrollable-dialog.tsx new file mode 100644 index 0000000000..4169cfaba7 --- /dev/null +++ b/packages/playground-react/pages/dialog/scrollable-dialog.tsx @@ -0,0 +1,90 @@ +import { Fragment, useRef, useState } from 'react' +import { Dialog, Transition } from '@headlessui/react' +import { ExclamationIcon } from '@heroicons/react/outline' + +export default function Example() { + const [open, setOpen] = useState(false) + + const cancelButtonRef = useRef(null) + + return ( + <> + + + + +
+ + +
+
+ + +
+
+
+
+ + Deactivate account + +
+

+ Are you sure you want to deactivate your account? All of your data will be + permanently removed from our servers forever. This action cannot be + undone. +

+ {Array(20) + .fill(null) + .map((_, i) => ( +

+ Are you sure you want to deactivate your account? All of your data + will be permanently removed from our servers forever. This action + cannot be undone. +

+ ))} +
+
+
+
+ + +
+ + +
+
+
+ + + ) +}