search

willmcpo / body-scroll-lock

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

touch scrolling on iOS will scroll the body BEHIND my modal (with enabled body scroll lock) #200

Open pbormodt opened 3 years ago

pbormodt commented 3 years ago

System: iOS 13

Description: I am building a modal which opens in a layer above the content and displays e.g. some text. It should lock the body scroll. Scrolling inside the modal should be still possible.

Problem: On iOS 13 there is a problem with nested elements inside the modal. If the scrollable content is at the beginning or the end and cannot be scrolled any further, it is possible to scroll the underlying body element by "scrolling"/touching in the direction without any content so scroll.

bodyScrollLock-scrolling-issue

I use the provided configuration example from the README. It works as described above that scrolling in nested elements on iOS is possible BUT with the described drawback of scrolling the body behind the modal ...

allowTouchMove: (el) => {
  while (el && el !== document.body) {
    console.log({ el });
    if (el.getAttribute("body-scroll-lock-ignore") !== null) {
      return true;
    }
    // Using `el.parentNode` as in the body-scroll-lock README threw a typescript error.
    // https://github.com/willmcpo/body-scroll-lock/#more-complex
    if (el.parentElement) {
      el = el.parentElement;
    }
  }
};

Is there anything I can do on my end to fix this issue or is it a bug in the library?

diachedelic commented 3 years ago

I think I may have noticed something like this before - is it related to scrolling momentum? I.e. is it only possible to scroll the body shortly after scrolling to the start/end of your modal?

pbormodt commented 3 years ago

I think I may have noticed something like this before - is it related to scrolling momentum? I.e. is it only possible to scroll the body shortly after scrolling to the start/end of your modal?

@diachedelic When I remove the momentum from css it's still not working — -webkit-overflow-scrolling: touch. Scrolling this way is possible the whole time. And only inside the element, I passed as a React ref to body-scroll-lock. So if I scroll in the header or footer of the modal, nothing happens (as supposed).

I noticed that adding overflow: hidden to the <body> on iOS seams to solve this issue. But I guess the overflow is not set for a specific reason on iOS.

For reference, in this scroll-lock library, nested elements do work as expected. But I am not able to tell the difference in the code.

pbormodt commented 3 years ago

I created a codesandbox with simplified modal code. Unfortunately the scrolling in this example does not work at all on iOS ... not sure why but maybe it helps visualizing what I work with roughly.

https://codesandbox.io/s/priceless-germain-ffrbs

EDIT: You can see what I described in the sandbox. At the top and at the bottom of the scroll container ModalContent you will move the body BEHIND the modal up and down. You can test it on an actual iPhone or the Simulator.

diachedelic commented 3 years ago

Thanks for the info - if you can reproduce your issue within that code sandbox I will take a closer look.

pbormodt commented 3 years ago

@diachedelic Updated the sandbox. It now shows the problem. Thank you in advance.

valleywood commented 3 years ago

I have the exact same problem. When I perform a touch gesture outside the modal the body starts scrolling and I need to click the modals scrollable area again (like I'm sort of activating it) before I'm able to scroll in the modal again.

diachedelic commented 3 years ago

Are you able to avoid using allowTouchMove? I think that is what's causing your problem - any element for which allowTouchMove returns true basically has the bodyScrollLock library disabled for it. And it looks like you are setting body-scroll-lock-ignore on your root Modal element.

valleywood commented 3 years ago

I need to allow touch move inside my modal since I have scrollable content inside my absolute positioned modal that covers 100% of the screen height and almost the full screen width (to make it behave like a drawer that slides in from the right) Found no other solution currently than using this:

On open modal

     if (!/iP(ad|hone|od)/.test(navigator.userAgent)) {
       // Use body scroll lock plugin
     } else {
       const iOSBodyStyle = {
         WebkitOverflowScrolling: 'none',
         overflow: 'hidden',
       };
       Object.assign(document.body.style, iOSBodyStyle);
     }

And then reset body-style attributes on modal close on iOS using the same technique. Works great on iOS 13 and onwards and with some small glitches on iOS <≈ 12

Hoping for a fix in the body-scroll-lock plugin so that I can remove this hack from my code 🙏

diachedelic commented 3 years ago

But can't you just do disableBodyScroll(".modal-content-inner"), without allowTouchMove?

pbormodt commented 3 years ago

Are you able to avoid using allowTouchMove?

@diachedelic No because I have nested elements inside the modal that needs to be scrollable -> The LayerContent. I dont see a reason not to use allowTouchMove as it is a feature of this library for working with nested elements.

And it looks like you are setting body-scroll-lock-ignore on your root Modal element.

@diachedelic I'm not sure if I understand correctly. I am setting this attribute on the element which needs to be scrollable.

function ModalContent({ children }: { children: React.ReactNode }) {
  return (
    <ModalContentRoot
      data-body-scroll-lock-ignore="true"
      data-id="modal-content"
    >
      <ModalContentInner data-id="modal-content-inner">
        {children}
      </ModalContentInner>
    </ModalContentRoot>
  );
}

I can't use disableBodyScroll directly on the ModalContent because it is inserted later by the developer:

I think that is what's causing your problem - any element for which allowTouchMove returns true basically has the bodyScrollLock library disabled for it.

@diachedelic Then this feature is not really usable? Or is the example in the README wrong? Else we have to rely on a hack like @valleywood described ...

Thanks for your support so far. I hope we find a solution.

diachedelic commented 3 years ago

I've only ever used allowTouchMove for things like textarea and input[type=range] elements - it's more for when you specifically need touchmove events to come through, not for scrolling a regular div. If I were you I would wait until ModalContent is available and then call disableBodyScroll on it. You can use the excellent MutationObserver if React does not provide a way.

pbormodt commented 3 years ago

Using some prop comparison on the react children and according to this assigning a ref to the ModalContent works in the sandbox example.

https://codesandbox.io/s/happy-burnell-43lem

const contentRef = useRef();
// ...
useBodyScrollLock(contentRef);
// ...
const resolveChildren = ({ children, ref }) => {
  const newChildren = React.Children.map(children, (child) => {
    if (!React.isValidElement(child)) return;

    if (typeof child.props.scrollable === "boolean" && child.props.scrollable) {
      return React.cloneElement(child, {
        ...child.props,
        ref
      });
    }

    return child;
  });

  return newChildren;
};
// ...
const children = resolveChildren({ children: childrenProp, ref: contentRef })
// ...
return (
  <div>
    {children}
  </div>
)

I have to check if this solution is also valid on my prod system. The last time I checked something did not work. I'll keep you updated

kalnode commented 3 years ago

Just providing our experience...

We use body-scroll-lock when we open a sidebar drawer menu. Body scrolling is locked successfully on all devices including iOS.

Problem: However, the scrolling of our target element (the sidebar drawer) didn't work on iOS (it worked everywhere else).

Ultimately, applying this on the target element was the key: -webkit-overflow-scrolling: touch

In addition, the child of the target (the content of our sidebar drawer) needed the following (however there's more CSS not shown here; sorry unable to put this up atm):

position: absolute;
height: 100vh;
width: 100%;
rick-liruixin commented 1 year ago

They stopped the repairs. I had to do it myself, in the same way, with a new version of typeScript. And fix these problems for everyone to use.

npm i body-scroll-lock-upgrade

repair log，Refer to the releases page.

rick-liruixin commented 1 year ago

They stopped the repairs. I had to do it myself, in the same way, with a new version of typeScript. And fix these problems for everyone to use.

npm i body-scroll-lock-upgrade

repair log，Refer to the releases page.