react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

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

Open basvandriel opened this issue 5 years ago โ€ข 17 comments

๐Ÿ› 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 the padding-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)

basvandriel avatar Nov 01 '20 21:11 basvandriel

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...

devongovett avatar Nov 01 '20 23:11 devongovett

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

basvandriel avatar Nov 02 '20 07:11 basvandriel

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

pascalduez avatar Feb 09 '21 15:02 pascalduez

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

snowystinger avatar Feb 18 '21 20:02 snowystinger

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

pascalduez avatar Feb 19 '21 17:02 pascalduez

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 avatar Apr 26 '21 21:04 snowystinger

@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 avatar Apr 26 '21 22:04 snowystinger

@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.

snowystinger avatar May 11 '21 01:05 snowystinger

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.

  1. 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 avatar Feb 10 '22 22:02 7iomka

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
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<K extends keyof GlobalEventHandlersEventMap>(
  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

  • For fixed headers you need just to add compensate-for-scrollbar className.
  • For sticky headers you need to check if their is in fixed state, and if not - remove padding
    .header.compensate-for-scrollbar:not(.is-sticky) {
     padding-right: 0;
    }
    
    or just dynamically add compensate-for-scrollbar className together with sticky className (in case of controlled component)

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.

7iomka avatar Feb 11 '22 00:02 7iomka

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

JeromeTrottier avatar Jun 02 '23 19:06 JeromeTrottier

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

xkomiks avatar Jul 28 '23 12:07 xkomiks

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

sleonia avatar Feb 07 '24 13:02 sleonia

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

sleonia avatar Feb 28 '24 15:02 sleonia

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";
    }
 }} />

signalkuppe avatar Mar 21 '24 09:03 signalkuppe

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;
}

predaytor avatar May 19 '24 21:05 predaytor

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

NickWoodward avatar Jun 25 '24 12:06 NickWoodward