mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
91.86k stars 31.57k forks source link

[Modal] Disable background scrolling in iOS #5750

Open tao-qian opened 7 years ago

tao-qian commented 7 years ago

Tested dialogs in http://www.material-ui.com/#/components/dialog. On desktop Chrome, background scrolling is disabled when dialogs are shown. However, it is not disabled in iOS Safari or Chrome.

sedletsky commented 7 years ago

And with Popover - when it's open you can scroll screen behind to negative position on iOS - very annoying...

Anybody did research or some related links?

Nopik commented 7 years ago

@oliviertassinari I'm being bitten by this bug, too. I can try to fix it, though at the moment I'm not even sure where to start. Do you have any idea what can be causing this?

mbrookes commented 7 years ago

@oliviertassinari Same problem on next, including full-screen dialogs.

oliviertassinari commented 6 years ago

As raised by someone on the bootstrap thread, that seems to be a Safari browser bug. We can't do much about it here. I'm closing the issue. It's unfortunate.

daniel-rabe commented 6 years ago

i had the issue with the popover component, so I added a custom BackdropComponent that cancels the touchmove event

import * as React from 'react';
import Backdrop, { BackdropProps } from 'material-ui/Modal/Backdrop';

/**
 * Prevents scrolling of content behind the backdrop.
 */
export class BackDropIOSWorkaround extends React.PureComponent<BackdropProps> {
    protected onTouchMove(event: React.TouchEvent<HTMLDivElement>): void {
        event.preventDefault();
    }

    public render(): JSX.Element {
        return (
            <Backdrop {...this.props} onTouchMove={this.onTouchMove}/>
        );
    }
}
<Popover
    BackdropInvisible={false}
    BackdropComponent={BackDropIOSWorkaround}
    anchorEl={this.clickElement}
    onRequestClose={this.unexpandChoices}
    anchorOrigin={{vertical: 'top', horizontal: 'left'}}
    transformOrigin={{vertical: 'top', horizontal: 'left'}}
>
    <List disablePadding={true}>
        {this.choices()}
    </List>
</Popover>
abcd-ca commented 6 years ago

@daniel-rabe 's solution looks good but would be for Material UI v1, not previous versions

jpmoyn commented 6 years ago

@oliviertassinari I see that the Dialog sets the style overflow-y: hidden; to the body. Would it be possible to add a Dialog prop that additionally sets the style position: fixed; on the body?

This would save a lot of people time in having to manually add position: fixed to the body when working with the Dialog component for mobile safari.

oliviertassinari commented 6 years ago

Would it be possible to add a Dialog prop that additionally sets the style position: fixed; on the body?

@jpmoyn No, you would reset the scroll position to the top of the page by doing so. Users will no longer be at the right scroll position once the dialog is closed.

jacobweber commented 6 years ago

Would it be possible to implement the onTouchMove workaround by default? I'm not sure it's possible to specify a custom BackdropComponent everywhere it's needed. This bug affects Dialog, Select, SwipeableDrawer, etc.

daniel-rabe commented 6 years ago

you can override the default-props of material-ui's BackDrop before creating your App:

import BackDrop from 'material-ui/Modal/Backdrop';
BackDrop.defaultProps = {...BackDrop.defaultProps, onTouchMove: preventBackdropScroll};

export function preventBackdropScroll(event: React.TouchEvent<HTMLElement>): void {
    let target: HTMLElement | null = (event.target as HTMLDivElement);
    while (target != null && target !== document.body) {
        const scrollHeight: number = target.scrollHeight;
        const clientHeight: number = target.clientHeight;
        if (scrollHeight > clientHeight) {
            return;
        }
        target = target.parentElement;
    }
    event.preventDefault();
}
jacobweber commented 6 years ago

@daniel-rabe Sneaky! I like it. Unfortunately it didn't fix the issue for me.

oliviertassinari commented 6 years ago

@jacobweber Are you saying https://github.com/mui-org/material-ui/issues/5750#issuecomment-351982758 workaround doesn't work? If it comes with no side effect, we could add it to the core of the library.

jacobweber commented 6 years ago

It didn't work for me (although I could see the function being invoked). Although maybe I'm doing something differently; I didn't get a chance to investigate this too deeply.

I also noticed that using -webkit-overflow-scrolling: touch in my scrollable views seems to trigger this behavior, for what it's worth.

daniel-rabe commented 6 years ago

the workaround does not work for current iOS, i dont know since when version exactly

daniel-rabe commented 6 years ago

this solution works for me atm

source: https://stackoverflow.com/questions/41594997/ios-10-safari-prevent-scrolling-behind-a-fixed-overlay-and-maintain-scroll-posi

import Fade from 'material-ui/transitions/Fade';

function fadeOnEnter(node: HTMLElement, isAppearing: boolean): void {
    let clientY: number | null = null; // remember Y position on touch start
    const touchStart: (event: Event) => void = (event: Event) => {
        if ((event as TouchEvent).targetTouches.length === 1) {
            clientY = (event as TouchEvent).targetTouches[0].clientY;
        }
    };
    const touchMove: (event: Event) => void = (event: Event) => {
        if ((event as TouchEvent).targetTouches.length === 1) {
            disableRubberBand(event as TouchEvent);
        }
    };
    const disableRubberBand: (event: TouchEvent) => void = (event: TouchEvent) => {
        const tmpClientY: number = event.targetTouches[0].clientY - (clientY || 0);

        if (node.scrollTop === 0 && tmpClientY > 0) {
            // element is at the top of its scroll
            event.preventDefault();
        }

        if (isOverlayTotallyScrolled() && tmpClientY < 0) {
            // element is at the top of its scroll
            event.preventDefault();
        }
    };
    const isOverlayTotallyScrolled: () => boolean = () => {
        // https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollHeight#Problems_and_solutions
        return node.scrollHeight - node.scrollTop <= node.clientHeight;
    };
    node.addEventListener('touchstart', touchStart, false);
    node.addEventListener('touchmove', touchMove, false);
}

Fade.defaultProps = {...Fade.defaultProps, onEnter: fadeOnEnter};
marzapower commented 6 years ago

@daniel-rabe I've tried your solution, but it's having (at least for me) the opposite effect: the background is scrollable while the Dialog content is no more. I'll try and investigate further ...

daniel-rabe commented 6 years ago

you are right, i did not noticed because my modal dialogs had no scrollable content.

this works for me for scrollable modals:

export function preventBackdropScroll(event: Event): void { let target: HTMLElement | null = (event.target as HTMLDivElement); while (target != null && target !== document.body) { const scrollHeight: number = target.scrollHeight; const clientHeight: number = target.clientHeight; if (scrollHeight > clientHeight) { return; } target = target.parentElement; } event.preventDefault(); }

export function disableOverflowScroll(node: HTMLElement): void { node.addEventListener('touchmove', preventBackdropScroll); }

import Fade from 'material-ui/transitions/Fade'; import Popover from 'material-ui/Popover';

Fade.defaultProps = {...Fade.defaultProps, onEnter: disableOverflowScroll}; Popover.defaultProps = {...Popover.defaultProps, onEntered: disableOverflowScroll};

... direct binding of preventBackdropScroll by react's onTouchMove does not work, I dont know why, event.preventDefault / event.nativeEvent.preventDefault has no effect on React.Synthetic-TouchEvents it seems

itwasmattgregg commented 5 years ago

You can write a listener for touchMove and stopPropagation on that event which will only scroll the modal. We're getting that to work pretty well

marzapower commented 5 years ago

The fact is that under certain circumstances on Safari iOS you can scroll the modal without firing the touchMove event (actually without firing any touch-related event).

Bosn commented 5 years ago

Come across the same issue on Drawer component. Desktop chrome works fine, but in iOS, the background page scrolls when draw menu scroll.

d-spiridonov commented 5 years ago

Fixed this issue by doing the following:

handleDialogOpen = () => {
    document.querySelector('#homePage').style.position = 'fixed'
    this.setState({
      dialogOpen: true,
    })
  }

  handleDialogClose = () => {
    document.querySelector('#homePage').style.position = 'static
    this.setState({
      dialogOpen: false
    })
  }
derakhshanfar commented 5 years ago

just use body-scroll-lock library and it works perfectly

dioxmio commented 5 years ago

yes, I think the https://github.com/willmcpo/body-scroll-lock should work. however shouldn't it com with this behaviour? Having the position:fixed is not a real solution since it will scroll the background to the top. that is a bad UX

dane-harnett commented 5 years ago

@derakhshanfar Do you have any examples of how to use body-scroll-lock with material-ui? I tried but it didn't actually work for me. Using a fullscreen Dialog.

stephen099 commented 5 years ago

@dane-harnett body-scroll-lock does not work for me either with material-ui

jantimon commented 5 years ago

@oliviertassinari would using https://github.com/willmcpo/body-scroll-lock be an option?

In that case I would try to provide a pull request

oliviertassinari commented 5 years ago

@jantimon I'm wondering. It comes with a non-negligible bundle size implication https://bundlephobia.com/result?p=body-scroll-lock@2.6.1: 1.1 kB. Does is worth is? Have you tried using this dependency by just using the public API of the Modal/Dialog?

I think that a demo integration in this issue is a good start!

oliviertassinari commented 5 years ago

A simple poc, to improve:

import { clearAllBodyScrollLocks, disableBodyScroll } from "body-scroll-lock";
import RootRef from "@material-ui/core/RootRef";

class Scrolllock extends React.Component {
  componentDidMount() {
    console.log("componentDidMount");
    clearAllBodyScrollLocks();
    disableBodyScroll(this.ref);
  }

  componentWillUnmount() {
    setTimeout(() => {
      clearAllBodyScrollLocks();
    }, 1000);
  }

  handleRef = ref => {
    this.ref = ref;
  };

  render() {
    return (
      <RootRef rootRef={this.handleRef}>
        <div>{this.props.children}</div>
      </RootRef>
    );
  }
}

https://codesandbox.io/s/1vwzvmo407

jantimon commented 5 years ago

Yes that sandbox looks great 👍

One thing we have to keep in mind is that if you open a model inside a model and close it again it would clear all locks - also the scroll lock for the parent modal.

The https://github.com/willmcpo/body-scroll-lock/blob/master/src/bodyScrollLock.js looks quite easy to use maybe we can reduce some lines which are not relevant for @material-ui however I believe most of that code will be required.

@material-ui already ships with a scroll-lock - do you think we could remove parts of it in favour of bodyScrollLock?

Did you already think about how @materia-ui/core could work together with dynamic imports? Maybe we could add a PopoverAsync component (which would not be in @material-ui/core/index similar to the ES modules). This component could lazy load @material-ui/core/Popover so the Popover code (popperjs, bodyScrollLock, and @material-ui/core/Popover) would not be part of the inital bundle and therefore would speed up the initial-load time.

oliviertassinari commented 5 years ago

@jantimon Yes, it's a quick POC for people that have basic needs, they can get away with it. We should probably add new hooks to the Modal to get it done correctly.

I'm not aware that dynamic import can be deployed to npm. As far as I know, it's bundler specific, not standardize. If there is a way, I'm happy to explore it. The alternative is to have another module, like a HOC people can wrap their modal/dialog with.

jantimon commented 5 years ago

Nice - let me know if I can do anything to support you with that :)

You are right with dynamic imports. Typescript will understand it out of the box but Babel will need an additional plugin. Furthermore the bundler has to translate the dynamic import into ajax code and do the code-splitting.

So building it right into @material-ui/core would probably cause many many issues.
I also thought about a tiny wrapper which has to be explicitly imported. E.g. import Popover from '@material-ui/core/PopoverAsync or import Popover from '@material-ui/async/Popover.

The code would probably be quite simple with a HOC or the React 16.6 lazy util:

import React, {Suspense} from 'react';
import { PopoverProps } from '@material-ui/core/PopoverProps';
const PopoverAsync = React.lazy(() => import('@material-ui/core/Popover'));
export default = (props: PopoverProps) => <Suspense fallback={''}><PopoverAsync ...props /></Suspense>
nmchaves commented 5 years ago

@oliviertassinari thanks for the POC! I'm having some difficulty getting it to work in iOS Safari and iOS Chrome 🤔. Here's what I see when I run the CodeSandbox and click "OPEN SIMPLE DIALOG"

screen shot 2018-12-10 at 11 46 18 am

I see the backdrop, but I don't see the dialog's body. I tried using the following iPhones: 1) iPhone 6s running iOS 11.2.5 (my own phone) 2) iPhone 8 running iOS 11 (using BrowserStack) 3) iPhone X running iOS 11 (using BrowserStack)

Note: I am able to open the dialogs correctly using the CodeSandbox examples from the docs. So the <Dialog/> component itself is working correctly.

@jantimon were you able to get the CodeSandbox example to work?

BrowserStack lets you interact with the DevTools, so I tried messing with a couple things. Interestingly, I could get the dialog's body to appear by removing: 1) the height: 100% style of .MuiDialog-container and/or 2) the display: flex style of .MuiDialog-scrollPaper

Neither of those tweaks are actual solutions though, because they mess up the alignment of the dialog's body.

joshwooding commented 5 years ago

I'm experiencing the same as @nmchaves

ghost commented 5 years ago

@oliviertassinari Thanks for the PoC, but it doesn't work when you have a scrollable list inside the Modal (example). Works fine in Safari on desktop, but not on iOS. You can't scroll the list at all 😞

hburrows commented 5 years ago

I'll chime in here and say it would be nice if either material-ui incorporated the functionality of body-scroll-lock or was more compatible with it -- at least if you're using body-scroll-lock inside a Modal. Modal sets overflow:hidden on document.body and if you're using body-scroll-lock (inside a Modal) overflow never gets restored when the modal is dismissed because body-scroll-lock restores overflow to the previous state which it always sees as hidden. To make matters worse body-scroll-lock restores the previous overflow setting in a delayed matter so you can't just simply call document.body.style.overflow = 'hidden' in the Dialog's componentWillUnmount.

A simple solution might be to just have an optional property to Modal which controls whether it sets overflow:hidden on document.body or not. That way, if you're using body-scroll-lock you can tell the Modal not to set overflow:hidden and let body-scroll-lock do all the work. Alternatively, and my preference, would be to incorporate body-scroll-lock (or its functionality) so another package isn't required.

This is also complicated by mobile Safari's handling of its bottom bar and if you're taking advantage of body scrolling and the bottom bar is hidden when the modal is opened. In that case you get all kinds of additional scroll wierdness which are completely unacceptable from a UX perspective. It's very messy. I can't help but think mobile Safari is the new IE.

cescoferraro commented 5 years ago

any pointers here at 3.9.2? I have tried all the above with no success. my whole app is based off fullScreen dialog. the only thing that actually works isthe snippet bellow but then I loose the scroll position on iOS

  React.useEffect(() => {
    const { style } = document.body
    const isIOS = ....
    if (isIOS) {
        style.position = open ? "fixed" : "unset"
    }
  }, [open])
svobik7 commented 5 years ago

First of all - Vote for integrating body-scroll-lock into MuiDialog.

Next I have got some quick workaround for @hburrows. You can reset body overflow before calling disableBodyScroll inside onEntered callback of your Dialog.

onEntered={() => {
  document.body.style.overflow = '';
  ref.current && disableBodyScroll(ref.current);
}}

This way I am able to restore scrolling body after Dialog is closed.

theKashey commented 5 years ago

There is no way you can use remove-body-scroll and React. @alexnez96 gave a good example when it does not work as expected.

I've forked given sandbox and used another library - react-remove-scroll, which uses React event propagation system and is friendly to any portals or any other react related stuff. Used in Reach-UI and Smooth-UI. Just 1.5kb (excluding tslib).

Here is a demo - https://codesandbox.io/s/ojq2m41jy6

PS: notice "-webkit-overflow-scrolling": "touch", - without it momentum scroll would be disabled.

oliviertassinari commented 5 years ago

@theKashey Thanks for sharing! So, the solution is to listen for the touch move event in the capturing phase and use a scroll detection logic close to what I do in react-swipeable-views to determine the correct course of action.

Speaking of the Modal, I would like to refactor the implementation. We can significantly reduce the size of the modal component. We can remove the withStyles imports and use a dead simple backdrop by default. We would go from 23 kB gzipped to 6.5kB gzipped: #15466.

joshwooding commented 5 years ago

overflow: hidden on iOS will be fixed soon https://bugs.webkit.org/show_bug.cgi?id=153852&utm_source=share&utm_medium=ios_app

oliviertassinari commented 5 years ago

Simon Fraser (smfr) 2019-05-07 13:28:53 PDT We don't make statements about future releases.

It might take a year to have a widely spread fix on iOS. Let's keep track of it.

joshwooding commented 4 years ago

It looks like they've fixed it (https://bugs.webkit.org/show_bug.cgi?id=153852#c29), we just need to wait for the fix to be released.

theKashey commented 4 years ago

The fix is covering the body scroll problem, but not:

While Modal case would be ok - some management still might be needed.

wereHamster commented 4 years ago

We've tested scroll blocking on latest iOS 13 beta, and it appears to be working on most pages that use overflow:hidden on body, but for example when you open the date pickers (https://material-ui-pickers.dev/demo/datepicker), which use Dialog internally, the scroll blocking does not work reliably. We couldn't reproduce it with a simple example, so there must be something that the date pickers / dialog do differently that still causes iOS to scroll.

oliviertassinari commented 4 years ago

@wereHamster Thank you for looking into it. Would you mind reporting the issue on https://github.com/mui-org/material-ui-pickers?

wereHamster commented 4 years ago

I don't think it's a bug in material-ui-pickers, we're having the same problem with MUI <TextField select>. To me it seems as the bugfix in WebKit is not always working and the bug resurfaces under certain conditions. I'll try to narrow it down to the smallest reproducible example tomorrow.

oliviertassinari commented 4 years ago

Safari's team says the bug was fixed in iOS 13, but it doesn't seem to be the case, at all: https://bugs.webkit.org/show_bug.cgi?id=153852.

I think that we could consider implementing a temporary patch, using these sources as inspiration:

theKashey commented 4 years ago

There are still "simple" way to handle it, setting position:fixed to the body (which also have other side effects, test thoughtfully)

oliviertassinari commented 4 years ago

@theKashey Does it reset the scroll position?

theKashey commented 4 years ago

Yes, but if you control "the thing" (aka modal) - you should be able to restore it properly. However, it might lead to some unwanted sideeffects :(