From 6f9de8925efdcf3f76e91ddd4bc80a2f343a9a6c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 2 Aug 2023 17:45:06 +0200 Subject: [PATCH] Disable smooth scrolling when opening/closing `Dialog` components on iOS (#2635) * disable smooth scrolling when opening/closing Dialogs For iOS workaround related purposes we have to capture the scroll position and offset the margin top with that amount and then `scrollTo(0,0,)` to prevent all kinds of funny UI jumps. However, if you have `scroll-behavior: smooth` enabled on your `html`, then offseting the margin-top and later `scrollTo(0,0)` would be handled in a smooth way, which means that the actual position would be off. To solve this, we disable smooth scrolling entirely in order to make the position of the Dialog correct. This shouldn't be a problem in practice since the page itself isn't suppose to scroll anyway. Once the Dialog closes we reset it such that everything else keeps working as expected in a (smooth) way. * add `microTask` to disposables * ensure the fix works in React's double rendering dev mode * update changelog --- packages/@headlessui-react/CHANGELOG.md | 1 + .../document-overflow/handle-ios-locking.ts | 142 ++++++++++-------- packages/@headlessui-vue/CHANGELOG.md | 1 + .../document-overflow/handle-ios-locking.ts | 16 ++ .../@headlessui-vue/src/utils/disposables.ts | 14 ++ 5 files changed, 112 insertions(+), 62 deletions(-) diff --git a/packages/@headlessui-react/CHANGELOG.md b/packages/@headlessui-react/CHANGELOG.md index 5537504bf0..1e327693ea 100644 --- a/packages/@headlessui-react/CHANGELOG.md +++ b/packages/@headlessui-react/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Use correct value when resetting `` and `` ([#2626](https://github.com/tailwindlabs/headlessui/pull/2626)) - Render `` in `Popover.Group` component only ([#2634](https://github.com/tailwindlabs/headlessui/pull/2634)) +- Disable smooth scrolling when opening/closing `Dialog` components on iOS ([#2635](https://github.com/tailwindlabs/headlessui/pull/2635)) ## [1.7.16] - 2023-07-27 diff --git a/packages/@headlessui-react/src/hooks/document-overflow/handle-ios-locking.ts b/packages/@headlessui-react/src/hooks/document-overflow/handle-ios-locking.ts index 71fb049ca8..c1df7b1411 100644 --- a/packages/@headlessui-react/src/hooks/document-overflow/handle-ios-locking.ts +++ b/packages/@headlessui-react/src/hooks/document-overflow/handle-ios-locking.ts @@ -1,3 +1,4 @@ +import { disposables } from '../../utils/disposables' import { isIOS } from '../../utils/platform' import { ScrollLockStep } from './overflow-store' @@ -24,74 +25,91 @@ export function handleIOSLocking(): ScrollLockStep { .some((container) => container.contains(el)) } - d.style(doc.body, 'marginTop', `-${scrollPosition}px`) - window.scrollTo(0, 0) + d.microTask(() => { + // We need to be able to offset the body with the current scroll position. However, if you + // have `scroll-behavior: smooth` set, then changing the scrollTop in any way shape or form + // will trigger a "smooth" scroll and the new position would be incorrect. + // + // This is why we are forcing the `scroll-behaviour: auto` here, and then restoring it later. + // We have to be a bit careful, because removing `scroll-behavior: auto` back to + // `scroll-behavior: smooth` can start triggering smooth scrolling. Delaying this by a + // microTask will guarantee that everything is done such that both enter/exit of the Dialog is + // not using smooth scrolling. + if (window.getComputedStyle(doc.documentElement).scrollBehavior !== 'auto') { + let _d = disposables() + _d.style(doc.documentElement, 'scroll-behavior', 'auto') + d.add(() => d.microTask(() => _d.dispose())) + } - // Relatively hacky, but if you click a link like `` in the Dialog, and there - // exists an element on the page (outside of the Dialog) with that id, then the browser will - // scroll to that position. However, this is not the case if the element we want to scroll to - // is higher and the browser needs to scroll up, but it doesn't do that. - // - // Let's try and capture that element and store it, so that we can later scroll to it once the - // Dialog closes. - let scrollToElement: HTMLElement | null = null - d.addEventListener( - doc, - 'click', - (e) => { - if (!(e.target instanceof HTMLElement)) { - return - } + d.style(doc.body, 'marginTop', `-${scrollPosition}px`) + window.scrollTo(0, 0) - try { - let anchor = e.target.closest('a') - if (!anchor) return - let { hash } = new URL(anchor.href) - let el = doc.querySelector(hash) - if (el && !inAllowedContainer(el as HTMLElement)) { - scrollToElement = el as HTMLElement + // Relatively hacky, but if you click a link like `` in the Dialog, and there + // exists an element on the page (outside of the Dialog) with that id, then the browser will + // scroll to that position. However, this is not the case if the element we want to scroll to + // is higher and the browser needs to scroll up, but it doesn't do that. + // + // Let's try and capture that element and store it, so that we can later scroll to it once the + // Dialog closes. + let scrollToElement: HTMLElement | null = null + d.addEventListener( + doc, + 'click', + (e) => { + if (!(e.target instanceof HTMLElement)) { + return } - } catch (err) {} - }, - true - ) - d.addEventListener( - doc, - '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 && !inAllowedContainer(e.target as HTMLElement)) { - e.preventDefault() - } - }, - { passive: false } - ) + try { + let anchor = e.target.closest('a') + if (!anchor) return + let { hash } = new URL(anchor.href) + let el = doc.querySelector(hash) + if (el && !inAllowedContainer(el as HTMLElement)) { + scrollToElement = el as HTMLElement + } + } catch (err) {} + }, + true + ) - // Restore scroll position - d.add(() => { - // Before opening the Dialog, we capture the current pageYOffset, and offset the page with - // this value so that we can also scroll to `(0, 0)`. - // - // If we want to restore a few things can happen: - // - // 1. The window.pageYOffset is still at 0, this means nothing happened, and we can safely - // restore to the captured value earlier. - // 2. The window.pageYOffset is **not** at 0. This means that something happened (e.g.: a - // link was scrolled into view in the background). Ideally we want to restore to this _new_ - // position. To do this, we can take the new value into account with the captured value from - // before. - // - // (Since the value of window.pageYOffset is 0 in the first case, we should be able to - // always sum these values) - window.scrollTo(0, window.pageYOffset + scrollPosition) + d.addEventListener( + doc, + '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 && !inAllowedContainer(e.target as HTMLElement)) { + e.preventDefault() + } + }, + { passive: false } + ) - // If we captured an element that should be scrolled to, then we can try to do that if the - // element is still connected (aka, still in the DOM). - if (scrollToElement && scrollToElement.isConnected) { - scrollToElement.scrollIntoView({ block: 'nearest' }) - scrollToElement = null - } + // Restore scroll position + d.add(() => { + // Before opening the Dialog, we capture the current pageYOffset, and offset the page with + // this value so that we can also scroll to `(0, 0)`. + // + // If we want to restore a few things can happen: + // + // 1. The window.pageYOffset is still at 0, this means nothing happened, and we can safely + // restore to the captured value earlier. + // 2. The window.pageYOffset is **not** at 0. This means that something happened (e.g.: a + // link was scrolled into view in the background). Ideally we want to restore to this _new_ + // position. To do this, we can take the new value into account with the captured value from + // before. + // + // (Since the value of window.pageYOffset is 0 in the first case, we should be able to + // always sum these values) + window.scrollTo(0, window.pageYOffset + scrollPosition) + + // If we captured an element that should be scrolled to, then we can try to do that if the + // element is still connected (aka, still in the DOM). + if (scrollToElement && scrollToElement.isConnected) { + scrollToElement.scrollIntoView({ block: 'nearest' }) + scrollToElement = null + } + }) }) }, } diff --git a/packages/@headlessui-vue/CHANGELOG.md b/packages/@headlessui-vue/CHANGELOG.md index 6a6ec9c9fa..50b50b49db 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 form elements for uncontrolled `` and `` ([#2626](https://github.com/tailwindlabs/headlessui/pull/2626)) - Use correct value when resetting `` and `` ([#2626](https://github.com/tailwindlabs/headlessui/pull/2626)) - Render `` in `PopoverGroup` component only ([#2634](https://github.com/tailwindlabs/headlessui/pull/2634)) +- Disable smooth scrolling when opening/closing `Dialog` components on iOS ([#2635](https://github.com/tailwindlabs/headlessui/pull/2635)) ## [1.7.15] - 2023-07-27 diff --git a/packages/@headlessui-vue/src/hooks/document-overflow/handle-ios-locking.ts b/packages/@headlessui-vue/src/hooks/document-overflow/handle-ios-locking.ts index 71fb049ca8..3954818283 100644 --- a/packages/@headlessui-vue/src/hooks/document-overflow/handle-ios-locking.ts +++ b/packages/@headlessui-vue/src/hooks/document-overflow/handle-ios-locking.ts @@ -1,3 +1,4 @@ +import { disposables } from '../../utils/disposables' import { isIOS } from '../../utils/platform' import { ScrollLockStep } from './overflow-store' @@ -24,6 +25,21 @@ export function handleIOSLocking(): ScrollLockStep { .some((container) => container.contains(el)) } + // We need to be able to offset the body with the current scroll position. However, if you + // have `scroll-behavior: smooth` set, then changing the scrollTop in any way shape or form + // will trigger a "smooth" scroll and the new position would be incorrect. + // + // This is why we are forcing the `scroll-behaviour: auto` here, and then restoring it later. + // We have to be a bit careful, because removing `scroll-behavior: auto` back to + // `scroll-behavior: smooth` can start triggering smooth scrolling. Delaying this by a + // microTask will guarantee that everything is done such that both enter/exit of the Dialog is + // not using smooth scrolling. + if (window.getComputedStyle(doc.documentElement).scrollBehavior !== 'auto') { + let _d = disposables() + _d.style(doc.documentElement, 'scroll-behavior', 'auto') + d.add(() => d.microTask(() => _d.dispose())) + } + d.style(doc.body, 'marginTop', `-${scrollPosition}px`) window.scrollTo(0, 0) diff --git a/packages/@headlessui-vue/src/utils/disposables.ts b/packages/@headlessui-vue/src/utils/disposables.ts index 855c697180..b71abd5f07 100644 --- a/packages/@headlessui-vue/src/utils/disposables.ts +++ b/packages/@headlessui-vue/src/utils/disposables.ts @@ -1,3 +1,5 @@ +import { microTask } from './micro-task' + export type Disposables = ReturnType export function disposables() { @@ -30,6 +32,18 @@ export function disposables() { api.add(() => clearTimeout(timer)) }, + microTask(...args: Parameters) { + let task = { current: true } + microTask(() => { + if (task.current) { + args[0]() + } + }) + return api.add(() => { + task.current = false + }) + }, + style(node: HTMLElement, property: string, value: string) { let previous = node.style.getPropertyValue(property) Object.assign(node.style, { [property]: value })