adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
13.04k stars 1.13k forks source link

React Aria's usePreventScroll hook causes whitespace shown on desktops from padding-right on html element #1216

Open basvandriel opened 4 years ago

basvandriel commented 4 years ago

๐Ÿ› Bug Report

When the usePreventScroll hook is used on a non-mobile device (with a scrollbar), a strip of whitespace shows up. This happens when a element with a 100% width and height is added to the DOM.

The problem is caused by thepadding-right that's added to the html element.

๐Ÿค” Expected Behavior

The whitespace should not be visible on the right side of the page.

๐Ÿ˜ฏ Current Behavior

The padding-right shows up at the right side of the page.

๐Ÿ’ Possible Solution

The body element styled should get overflow: hidden assigned. Also possible to give the body element the same background color as the fullscreen container, which is not optimal.

๐Ÿ”ฆ Context

See the right side of the page, used in a new Next.js application.

image

๐Ÿ’ป Code Sample

See the Github Gist here.

๐ŸŒ Your Environment

Software Version(s)
react-aria 3.1.0
Browser Google Chrome
Operating System Windows 10 64bit

๐Ÿงข Your Company/Team

๐Ÿ•ท Tracking Issue (optional)

devongovett commented 4 years ago

This is intentional to offset the size of the scrollbar and prevent the page from shifting when scrolling is disabled. I'm not sure there is a way to avoid the whitespace...

basvandriel commented 4 years ago

I figured it's intentional. When adding the overflow: hidden to the body the issue solves itselves.

pascalduez commented 3 years ago

I have a similar issue, prior to using @react-aria I had my own prevent scroll implementation and used to disable overflow + add padding on the body element, now it's set on the root element. With fixed elements like headers the padding doesn't work obviously.

Would adding some sort of callback to usePreventcroll be something conceivable? So that one could react and make more tweaks in sync with those changes.

https://user-images.githubusercontent.com/335467/107388355-0e56ca00-6af6-11eb-8952-f61a308e1f2c.mp4

snowystinger commented 3 years ago

As commented on the PR attached to this, we'll be taking a look after this sprint. @pascalduez please let us know if your app is not fixed by that proposed change

pascalduez commented 3 years ago

Hi @snowystinger, thanks for your feedback.

I don't think the change in #1278 is helping in my case.
Actually I have both issues: moving header content, right white space.

See a reproduction here https://codesandbox.io/s/red-violet-pm7z0?file=/src/App.js

snowystinger commented 3 years ago

Hey, sorry about the delay, but I'm actually unable to verify that overflow hidden on the body solves the issue as you've stated. I've done my best to reproduce it here https://codesandbox.io/s/awesome-cache-oj4g1?file=/src/App.js

snowystinger commented 3 years ago

@pascalduez I see your issue, you're using position fixed. Can you put a width on the parent of the position fixed (you may have to put it on some elements further up the tree as well). The end goal being to use width: inherit on the position fixed element. If it's possible, great! Otherwise I'm discussing other options with the team.

snowystinger commented 3 years ago

@pascalduez we'll need to find some other way than a callback for usePreventScroll, that'd potentially require listening to every dialog etc that makes use of usePreventScroll, in a large shared code base, this could be untenable. We don't have another idea at this time, so all ideas are welcome.

7iomka commented 2 years ago

I think the best way to solve this issue is following steps 1) Instead of inline styles to html element add custom className compensate-for-scrollbar 2) Add this className to body element when isActive, AND generate styles for this className

<style>
:root {
 --scrollbar-width: 15px; // dynamic value generated here
}
.compensate-for-scrollbar {
    padding-right: var(--scrollbar-width);
}
body.compensate-for-scrollbar {
    overflow: hidden !important;
    touch-action: none;
}
</style>

When is not active - nothing.

3) For fix jump for fixed elements like headers, users can use the same className on this elements to allow scrollbar-padding-fix be applied. Important note: for fullwidth fixed elements instead of width: 100% use left: 0; right:0 to work correctly with padding compensation.

This technique is well known and used in multiple popular libraries like fancybox.

At this moment I don't have any solution to do the same thing with usePreventScroll as I described to achieve the same result. I know this hook as a whole aria library is not related to styling, but I think this is a good step towards users.

7iomka commented 2 years ago

I looked at the source code of the hook and came to the conclusion that, in principle, it is enough for the user that the value be placed in the style variable, then user can manually create .compensate-for-scrollbar className with css var reference in the value.

Here is the modified code

usePreventScroll - refactored ```ts import { chain, getScrollParent, isIOS, useLayoutEffect } from '@react-aria/utils'; interface PreventScrollOptions { /** Whether the scroll lock is disabled. */ isDisabled?: boolean; } // @ts-ignore const visualViewport = typeof window !== 'undefined' && window.visualViewport; // HTML input types that do not cause the software keyboard to appear. const nonTextInputTypes = new Set([ 'checkbox', 'radio', 'range', 'color', 'file', 'image', 'button', 'submit', 'reset', ]); /** * Prevents scrolling on the document body on mount, and * restores it on unmount. Also ensures that content does not * shift due to the scrollbars disappearing. */ export function usePreventScroll(options: PreventScrollOptions = {}) { let { isDisabled } = options; useLayoutEffect(() => { if (isDisabled) { return; } if (isIOS()) { return preventScrollMobileSafari(); } else { return preventScrollStandard(); } }, [isDisabled]); } // For most browsers, all we need to do is set `overflow: hidden` on the root element, and // add some padding to prevent the page from shifting when the scrollbar is hidden. function preventScrollStandard() { const scrollbarWidth = `${window.innerWidth - document.documentElement.clientWidth}px`; return chain( setStyle(document.documentElement, '--scrollbar-width', scrollbarWidth, true), setStyle(document.documentElement, 'paddingRight', 'var(--scrollbar-width)'), setStyle(document.documentElement, 'overflow', 'hidden'), ); } // Mobile Safari is a whole different beast. Even with overflow: hidden, // it still scrolls the page in many situations: // // 1. When the bottom toolbar and address bar are collapsed, page scrolling is always allowed. // 2. When the keyboard is visible, the viewport does not resize. Instead, the keyboard covers part of // it, so it becomes scrollable. // 3. When tapping on an input, the page always scrolls so that the input is centered in the visual viewport. // This may cause even fixed position elements to scroll off the screen. // 4. When using the next/previous buttons in the keyboard to navigate between inputs, the whole page always // scrolls, even if the input is inside a nested scrollable element that could be scrolled instead. // // In order to work around these cases, and prevent scrolling without jankiness, we do a few things: // // 1. Prevent default on `touchmove` events that are not in a scrollable element. This prevents touch scrolling // on the window. // 2. Prevent default on `touchmove` events inside a scrollable element when the scroll position is at the // top or bottom. This avoids the whole page scrolling instead, but does prevent overscrolling. // 3. Prevent default on `touchend` events on input elements and handle focusing the element ourselves. // 4. When focusing an input, apply a transform to trick Safari into thinking the input is at the top // of the page, which prevents it from scrolling the page. After the input is focused, scroll the element // into view ourselves, without scrolling the whole page. // 5. Offset the body by the scroll position using a negative margin and scroll to the top. This should appear the // same visually, but makes the actual scroll position always zero. This is required to make all of the // above work or Safari will still try to scroll the page when focusing an input. // 6. As a last resort, handle window scroll events, and scroll back to the top. This can happen when attempting // to navigate to an input with the next/previous buttons that's outside a modal. function preventScrollMobileSafari() { let scrollable: Element; let lastY = 0; let onTouchStart = (e: TouchEvent) => { // Store the nearest scrollable parent element from the element that the user touched. scrollable = getScrollParent(e.target as Element); if (scrollable === document.documentElement && scrollable === document.body) { return; } lastY = e.changedTouches[0].pageY; }; let onTouchMove = (e: TouchEvent) => { // Prevent scrolling the window. if (scrollable === document.documentElement || scrollable === document.body) { e.preventDefault(); return; } // Prevent scrolling up when at the top and scrolling down when at the bottom // of a nested scrollable area, otherwise mobile Safari will start scrolling // the window instead. Unfortunately, this disables bounce scrolling when at // the top but it's the best we can do. let y = e.changedTouches[0].pageY; let scrollTop = scrollable.scrollTop; let bottom = scrollable.scrollHeight - scrollable.clientHeight; if ((scrollTop <= 0 && y > lastY) || (scrollTop >= bottom && y < lastY)) { e.preventDefault(); } lastY = y; }; let onTouchEnd = (e: TouchEvent) => { let target = e.target as HTMLElement; if (target instanceof HTMLInputElement && !nonTextInputTypes.has(target.type)) { e.preventDefault(); // Apply a transform to trick Safari into thinking the input is at the top of the page // so it doesn't try to scroll it into view. When tapping on an input, this needs to // be done before the "focus" event, so we have to focus the element ourselves. target.style.transform = 'translateY(-2000px)'; target.focus(); requestAnimationFrame(() => { target.style.transform = ''; }); } }; let onFocus = (e: FocusEvent) => { let target = e.target as HTMLElement; if (target instanceof HTMLInputElement && !nonTextInputTypes.has(target.type)) { // Transform also needs to be applied in the focus event in cases where focus moves // other than tapping on an input directly, e.g. the next/previous buttons in the // software keyboard. In these cases, it seems applying the transform in the focus event // is good enough, whereas when tapping an input, it must be done before the focus event. ๐Ÿคทโ€โ™‚๏ธ target.style.transform = 'translateY(-2000px)'; requestAnimationFrame(() => { target.style.transform = ''; // This will have prevented the browser from scrolling the focused element into view, // so we need to do this ourselves in a way that doesn't cause the whole page to scroll. if (visualViewport) { if (visualViewport.height < window.innerHeight) { // If the keyboard is already visible, do this after one additional frame // to wait for the transform to be removed. requestAnimationFrame(() => { scrollIntoView(target); }); } else { // Otherwise, wait for the visual viewport to resize before scrolling so we can // measure the correct position to scroll to. visualViewport.addEventListener('resize', () => scrollIntoView(target), { once: true }); } } }); } }; let onWindowScroll = () => { // Last resort. If the window scrolled, scroll it back to the top. // It should always be at the top because the body will have a negative margin (see below). window.scrollTo(0, 0); }; // Record the original scroll position so we can restore it. // Then apply a negative margin to the body to offset it by the scroll position. This will // enable us to scroll the window to the top, which is required for the rest of this to work. let scrollX = window.pageXOffset; let scrollY = window.pageYOffset; const scrollbarWidth = `${window.innerWidth - document.documentElement.clientWidth}px`; let restoreStyles = chain( setStyle(document.documentElement, '--scrollbar-width', scrollbarWidth, true), setStyle(document.documentElement, 'paddingRight', 'var(--scrollbar-width)'), setStyle(document.documentElement, 'overflow', 'hidden'), setStyle(document.body, 'marginTop', `-${scrollY}px`), ); // Scroll to the top. The negative margin on the body will make this appear the same. window.scrollTo(0, 0); let removeEvents = chain( addEvent(document, 'touchstart', onTouchStart, { passive: false, capture: true }), addEvent(document, 'touchmove', onTouchMove, { passive: false, capture: true }), addEvent(document, 'touchend', onTouchEnd, { passive: false, capture: true }), addEvent(document, 'focus', onFocus, true), addEvent(window, 'scroll', onWindowScroll), ); return () => { // Restore styles and scroll the page back to where it was. restoreStyles(); removeEvents(); window.scrollTo(scrollX, scrollY); }; } // Sets a CSS property on an element, and returns a function to revert it to the previous value. function setStyle(element: HTMLElement, style: string, value: string, isCssProp: boolean) { let cur = isCssProp ? element.style.getPropertyValue(style) : element.style[style]; if (isCssProp) { element.style.setProperty(style, value); } else { element.style[style] = value; } return () => { if (isCssProp) { element.style.setProperty(style, cur); } else { element.style[style] = cur; } }; } // Adds an event listener to an element, and returns a function to remove it. function addEvent( target: EventTarget, event: K, handler: (this: Document, ev: GlobalEventHandlersEventMap[K]) => any, options?: boolean | AddEventListenerOptions, ) { target.addEventListener(event, handler, options); return () => { target.removeEventListener(event, handler, options); }; } function scrollIntoView(target: Element) { // Find the parent scrollable element and adjust the scroll position if the target is not already in view. let scrollable = getScrollParent(target); if (scrollable !== document.documentElement && scrollable !== document.body) { let scrollableTop = scrollable.getBoundingClientRect().top; let targetTop = target.getBoundingClientRect().top; if (targetTop > scrollableTop + target.clientHeight) { scrollable.scrollTop += targetTop - scrollableTop; } } } ```

Somewhere in your global styles

.compensate-for-scrollbar {
 padding-right: var(--scrollbar-width);
}

Then

That fix for me problems related with jumping fixed/sticky elements. Regarding the issue of changing the placement of styles I don't have specific ideas why you are doing this on html and not on the body, but for my cases this is enough.

JeromeTrottier commented 1 year ago

Any updates on this issue? It seems to still be a current thing

xkomiks commented 1 year ago

react-spectrum still making progress? the best solution has already been provided above with compensation, but it's been a year

sleonia commented 9 months ago

@snowystinger Hi there. Could you please kindly provide the status update on the issue, if it's not too much trouble?

sleonia commented 8 months ago

@devongovett @pascalduez @basvandriel @7iomka https://github.com/adobe/react-spectrum/issues/5470#issuecomment-1969262184

signalkuppe commented 7 months ago

same issue here, used this workaround for the DateRangePicker

<DateRangePicker
 value={value}
 onChange={onChange}
 onOpenChange={(isOpen) => {
    if (isOpen) {
      document.documentElement.style.marginRight = "-15px";
    } else {
      document.documentElement.style.marginRight = "0px";
    }
 }} />
predaytor commented 5 months ago

Persist scrollbar on html element (not the body) and set <ModalOverlay /> styles:

html {
    overflow-block: scroll;
}

.modalOverlay {
    scrollbar-gutter: stable;
    position: fixed;
    block-size: var(--visual-viewport-height);
    inline-size: 100vw;
}
NickWoodward commented 4 months ago

So if your fixed element header is jumping, this: pl-[calc(100vw-100%)] should stop this (you may have to add w-[100vw] too)

For your content you also need to add 100vw and overflow-x hidden.

Demo: https://codesandbox.io/p/devbox/tmsykz?file=%2Fsrc%2Fstyles.css%3A6%2C1-6%2C5