From 9d8efb112cb1cafc4cd377351f100e2b93ac04ca Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 15 Dec 2022 15:44:30 +0100 Subject: [PATCH 1/3] improve types for addEventListener inside disposables --- packages/@headlessui-react/src/utils/disposables.ts | 2 +- packages/@headlessui-vue/src/utils/disposables.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@headlessui-react/src/utils/disposables.ts b/packages/@headlessui-react/src/utils/disposables.ts index 686f4fff5..ef505c441 100644 --- a/packages/@headlessui-react/src/utils/disposables.ts +++ b/packages/@headlessui-react/src/utils/disposables.ts @@ -10,7 +10,7 @@ export function disposables() { }, addEventListener( - element: HTMLElement | Document, + element: HTMLElement | Window | Document, name: TEventName, listener: (event: WindowEventMap[TEventName]) => any, options?: boolean | AddEventListenerOptions diff --git a/packages/@headlessui-vue/src/utils/disposables.ts b/packages/@headlessui-vue/src/utils/disposables.ts index 8e81cafe3..70667ce91 100644 --- a/packages/@headlessui-vue/src/utils/disposables.ts +++ b/packages/@headlessui-vue/src/utils/disposables.ts @@ -8,7 +8,7 @@ export function disposables() { }, addEventListener( - element: HTMLElement | Document, + element: HTMLElement | Window | Document, name: TEventName, listener: (event: WindowEventMap[TEventName]) => any, options?: boolean | AddEventListenerOptions From 1c88b4645ca9b556ea16b2f13fcc19d77c242178 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 15 Dec 2022 15:44:52 +0100 Subject: [PATCH 2/3] improve scroll locking Instead of using the "simple" hack with the `position: fixed;` we now went back to the `touchmove` implementation. The `position: fixed;` causes some annoying issues. For starters, on iOS you will now get a strange gap (due to safe areas). Some applications also saw "blank" screens based on how the page was implemented. We also saw some issues internally, where clicking changing the scroll position on the main page from within the Dialog. Think about something along the lines of: ```html Interesting link on the page ``` This doesn't work becauase the page is now fixed, and there is nothing to scroll... Instead, we now use the `touchmove` again. The problem with this last time was that this disabled _all_ touch move events. This is obviously not good. Luckily, we already have a concept of "safe containers". This is what we use for the `outside click` behaviour as well. Basically in a Dialog, your `Dialog.Panel` is the safe container. But also third party DOM elements that are rendered inside that Panel (or as a sibling of the Dialog, but not your main app). We can re-use this knowledge of "safe containers", and only cancel the `touchmove` behaviour if this didn't happen in any of the safe containers. --- .../src/components/dialog/dialog.tsx | 71 ++++++++++++------- .../src/components/dialog/dialog.ts | 54 +++++++++----- 2 files changed, 83 insertions(+), 42 deletions(-) diff --git a/packages/@headlessui-react/src/components/dialog/dialog.tsx b/packages/@headlessui-react/src/components/dialog/dialog.tsx index f346bf76c..dd1729ebb 100644 --- a/packages/@headlessui-react/src/components/dialog/dialog.tsx +++ b/packages/@headlessui-react/src/components/dialog/dialog.tsx @@ -91,7 +91,11 @@ function useDialogContext(component: string) { return context } -function useScrollLock(ownerDocument: Document | null, enabled: boolean) { +function useScrollLock( + ownerDocument: Document | null, + enabled: boolean, + resolveAllowedContainers: () => HTMLElement[] = () => [document.body] +) { useEffect(() => { if (!enabled) return if (!ownerDocument) return @@ -120,9 +124,27 @@ function useScrollLock(ownerDocument: Document | null, enabled: boolean) { if (isIOS()) { let scrollPosition = window.pageYOffset - style(documentElement, 'position', 'fixed') - style(documentElement, 'marginTop', `-${scrollPosition}px`) - style(documentElement, 'width', `100%`) + style(document.body, 'marginTop', `-${scrollPosition}px`) + window.scrollTo(0, 0) + + d.addEventListener( + ownerDocument, + 'touchmove', + (e) => { + // Check if we are scrolling inside any of the allowed containers, if not let's cancel the event! + if ( + e.target instanceof HTMLElement && + !resolveAllowedContainers().some((container) => + container.contains(e.target as HTMLElement) + ) + ) { + e.preventDefault() + } + }, + { passive: false } + ) + + // Restore scroll position d.add(() => window.scrollTo(0, scrollPosition)) } @@ -242,27 +264,22 @@ let DialogRoot = forwardRefWithAs(function Dialog< // Ensure other elements can't be interacted with useInertOthers(internalDialogRef, hasNestedDialogs ? enabled : false) - // Close Dialog on outside click - useOutsideClick( - () => { - // Third party roots - 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 - }) + let resolveContainers = useEvent(() => { + // Third party roots + 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[] - }, - close, - enabled && !hasNestedDialogs - ) + return [...rootContainers, state.panelRef.current ?? internalDialogRef.current] as HTMLElement[] + }) + + // Close Dialog on outside click + useOutsideClick(() => resolveContainers(), close, enabled && !hasNestedDialogs) // Handle `Escape` to close useEventListener(ownerDocument?.defaultView, 'keydown', (event) => { @@ -276,7 +293,11 @@ let DialogRoot = forwardRefWithAs(function Dialog< }) // Scroll lock - useScrollLock(ownerDocument, dialogState === DialogStates.Open && !hasParentDialog) + useScrollLock( + ownerDocument, + dialogState === DialogStates.Open && !hasParentDialog, + resolveContainers + ) // Trigger close when the FocusTrap gets hidden useEffect(() => { diff --git a/packages/@headlessui-vue/src/components/dialog/dialog.ts b/packages/@headlessui-vue/src/components/dialog/dialog.ts index d59767b71..9041df4a9 100644 --- a/packages/@headlessui-vue/src/components/dialog/dialog.ts +++ b/packages/@headlessui-vue/src/components/dialog/dialog.ts @@ -183,22 +183,23 @@ export let Dialog = defineComponent({ provide(DialogContext, api) - // Handle outside click - useOutsideClick( - () => { - // Third party roots - let rootContainers = Array.from( - ownerDocument.value?.querySelectorAll('body > *, [data-headlessui-portal]') ?? [] - ).filter((container) => { - if (!(container instanceof HTMLElement)) return false // Skip non-HTMLElements - if (container.contains(dom(mainTreeNode))) return false // Skip if it is the main app - if (api.panelRef.value && container.contains(api.panelRef.value)) return false - return true // Keep - }) + function resolveAllowedContainers() { + // Third party roots + let rootContainers = Array.from( + ownerDocument.value?.querySelectorAll('body > *, [data-headlessui-portal]') ?? [] + ).filter((container) => { + if (!(container instanceof HTMLElement)) return false // Skip non-HTMLElements + if (container.contains(dom(mainTreeNode))) return false // Skip if it is the main app + if (api.panelRef.value && container.contains(api.panelRef.value)) return false + return true // Keep + }) - return [...rootContainers, api.panelRef.value ?? internalDialogRef.value] as HTMLElement[] - }, + return [...rootContainers, api.panelRef.value ?? internalDialogRef.value] as HTMLElement[] + } + // Handle outside click + useOutsideClick( + () => resolveAllowedContainers(), (_event, target) => { api.close() nextTick(() => target?.focus()) @@ -249,9 +250,28 @@ export let Dialog = defineComponent({ if (isIOS()) { let scrollPosition = window.pageYOffset - style(documentElement, 'position', 'fixed') - style(documentElement, 'marginTop', `-${scrollPosition}px`) - style(documentElement, 'width', `100%`) + style(document.body, 'marginTop', `-${scrollPosition}px`) + window.scrollTo(0, 0) + + d.addEventListener( + owner, + 'touchmove', + (e) => { + // Check if we are scrolling inside any of the allowed containers, if not let's cancel + // the event! + if ( + e.target instanceof HTMLElement && + !resolveAllowedContainers().some((container) => + container.contains(e.target as HTMLElement) + ) + ) { + e.preventDefault() + } + }, + { passive: false } + ) + + // Restore scroll position d.add(() => window.scrollTo(0, scrollPosition)) } From 20f9c50cd101978cb2480b3459cd3f0f26f5e50f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Thu, 15 Dec 2022 15:53:56 +0100 Subject: [PATCH 3/3] update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + packages/@headlessui-vue/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index f96c0f13a..8a1f8cba7 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix regression where `displayValue` crashes ([#2087](https://github.com/tailwindlabs/headlessui/pull/2087)) - Fix `displayValue` syncing when `Combobox.Input` is unmounted and re-mounted in different trees ([#2090](https://github.com/tailwindlabs/headlessui/pull/2090)) - Fix FocusTrap escape due to strange tabindex values ([#2093](https://github.com/tailwindlabs/headlessui/pull/2093)) +- Improve scroll locking on iOS ([#2100](https://github.com/tailwindlabs/headlessui/pull/2100)) ## [1.7.5] - 2022-12-08 diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 558ddf10f..9678ea0ba 100644 --- a/packages/@headlessui-vue/CHANGELOG.md +++ b/packages/@headlessui-vue/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fix regression where `displayValue` crashes ([#2087](https://github.com/tailwindlabs/headlessui/pull/2087)) - Fix `displayValue` syncing when `Combobox.Input` is unmounted and re-mounted in different trees ([#2090](https://github.com/tailwindlabs/headlessui/pull/2090)) - Fix FocusTrap escape due to strange tabindex values ([#2093](https://github.com/tailwindlabs/headlessui/pull/2093)) +- Improve scroll locking on iOS ([#2100](https://github.com/tailwindlabs/headlessui/pull/2100)) ## [1.7.5] - 2022-12-08