search

willmcpo / body-scroll-lock

Body scroll locking that just works with everything 😏
MIT License
4.05k stars 338 forks source link

Does not work in gatsby projects #154

Open soosap opened 4 years ago

soosap commented 4 years ago

In order to lock the scrolling in a gatsby project, the overflow: hidden style property must be set on the root <html style="overflow: hidden;"> element.

iamskok commented 4 years ago

@soosap can you please provide more information on this issue?

kpratik2015 commented 4 years ago

Basically this isn't working on gatsby => enableBodyScroll(document.body); The body does get overflow hidden but the scrolling isn't stopped.

jamesirving commented 4 years ago

same issue here for my gatsby project... style="overflow: hidden;" is applied to the body element but this does not prevent scrolling.

diachedelic commented 4 years ago

I know nothing about Gatsby, but assuming gatsbyjs.org is developed using Gatsby, it forces the <html> element to scroll:

image

aidan-rypens commented 4 years ago

Someone made it work on gatsby?

diachedelic commented 4 years ago

@AidanRRR Can you try replacing all instances of document.body with document.documentElement in body-scroll-lock and see if that helps?

aidan-rypens commented 4 years ago

It works, thanks!

mdhornet90 commented 4 years ago

Confirmed this fixes my issue in a gatsby project as well, I have to hide overflow in the root in componentDidMount and delete that in componentWillUnmount. Any consideration for how this might still be incorporated into gatsby projects in a more general way?

diachedelic commented 4 years ago

@mdhornet90 by root do you mean the <html> element?

Are you sure you replaced the document.body on this line? It should apply overflow on the <html> element if you did.

mdhornet90 commented 4 years ago

Yes, I'm referring to the <html> element. I didn't want to modify the library in place or fork a custom version of BSL so this code:

  componentDidMount() {
    this.scrollRef = this.scrollRef || React.createRef();
    document.documentElement.style.overflow = 'hidden';
    disableBodyScroll(this.scrollRef.current);
  }

  componentWillUnmount() {
    clearAllBodyScrollLocks();
    document.documentElement.style.overflow = null;
  }

Does the trick for my specific situation (overlay that prevents scrolling of everything except the content of the overlay)

diachedelic commented 4 years ago

Well, perhaps we could restrict overflow of both <body> and <html>...can you think of any issues with this @willmcpo ?

kpratik2015 commented 4 years ago

I had to do this:

// for disabling
React.useEffect(() => {
    document.getElementsByTagName("html")[0].style.overflow = "hidden";
    disableBodyScroll(document.body);
  }, []);

// for enabling
React.useEffect(() => {
    enableBodyScroll(document.body);
    document.getElementsByTagName("html")[0].style = "";
  }, []);
WickyNilliams commented 3 years ago

I am not using gatsby, though I have noticed the same issues when doing SSR where disableBodyScroll is called on the server, and then enableBodyScroll on client.

When calling enableBodyScroll, this lib checks its previousBodyOverflowSetting and previousBodyPaddingRight variables to know what to revert to re-enabling scrolling. However those variables will never be set when the environment in which scrolling is disabled (the server doing SSR) is different to the environment where scrolling is enabled (the browser)

I imagine this issue is the same for gatsby, where scroll is disabled at build time, and enabled on the client.

I propose: instead of storing previous values in a variable, store in a data-* attribute on the body, since these will be persisted across server/client (or build-time/run-time). I have tested this locally and it seems to work. I can make a PR if you like

diachedelic commented 3 years ago

@WickyNilliams why is disableBodyScroll called in an SSR environment?

@lizy0329 A solution for this might be to change body-scroll-lock to prevent overflow on both document.body and document.documentElement. I don't have time to do that right now, but if anyone would like to try and test that approach then submit a PR that would be great!

WickyNilliams commented 3 years ago

@diachedelic excuse the delay, forgot to respond!

It's called in server because I may want to show a modal pre-opened on page load. I'm sure there are plenty of other use-cases besides.

In any case, whether calling at build-time (SSG) and server-side (SSR) the problem is the same - the assumption that enabling and disabling scrolling will always happen in the same environment

Hopefully I am making sense here :)

diachedelic commented 3 years ago

@WickyNilliams If I were you I would avoid locking the scroll in anything but a browser environment, as it just does not make sense to do so on the server. Is a way for you to determine which environment you are in?

WickyNilliams commented 3 years ago

@diachedelic shall i create a separate issue to discuss? I feel I am distracting from the original issue here (i misread originally, which is why i commented here, but this issue is not related to my question)...

I think there are valid reasons to call on the server, and from a dev perspective the less you have to special-case the server environment, the better. I have a few ideas in mind that could work that would be small changes, and backwards compatible.

diachedelic commented 3 years ago

@WickyNilliams Yes, this belongs in a seperate issue. But I think you need to understand that disableScrollLock adds event listeners etc, it does not just modify the DOM. It just does not make sense to call it in a server environment. You should call it immediately after "rehydration" if you need a modal to appear immediately.

hellodavecooper commented 3 years ago

I was also having this problem, in Gatsby v2.32.10, with body-scroll-lock v3.1.5. My requirement was to lock the scroll when a modal side-navigation "drawer" was open.

The solution in https://github.com/willmcpo/body-scroll-lock/issues/154#issuecomment-612664227 improved the situation, but I also had to dynamically set the height style of the #gatsby-focus-wrapper div, like this:

  const [isOpen, setIsOpen] = useState(false); // represents the open state of the navigation drawer
  const headerEl = useRef();

  useEffect(() => {
    if (typeof window === 'undefined') return;
    const gatsbyDiv = document.getElementById('gatsby-focus-wrapper');

    if (isOpen) {
      gatsbyDiv.style.height = '100%';
      document.documentElement.style.overflow = 'hidden';

      disableBodyScroll(headerEl.current);
    } else {
      gatsbyDiv.style.height = 'auto';
      document.documentElement.style.overflow = 'auto';

      enableBodyScroll(headerEl.current);
    }

    return clearAllBodyScrollLocks();
  }, [isOpen]);
yuittti commented 2 years ago

@hellodavecooper It's more likely there could be a potential issue with one part of your solution - you are setting overflow to auto on closing but auto isn't a default value of overflow and it has a little bit different behavior. I suggest setting overflow to visible as it is a default value of this property.