Fix scroll lock for iOS devices

jvitela commented 3 years ago

As commented here: https://github.com/willmcpo/body-scroll-lock/issues/200#issuecomment-692494479 Other libraries successfully use this strategy on iOS devices, this could be an opt-in to solve many iOS related issues reported here.

jtomaszewski commented 3 years ago

When should one use hideBodyOverflow: true and hideBodyOverflow: false ? (What are the drawbacks of using it?)

Is it possible to provide a sensible default value that works for everybody in all cases?

jvitela commented 3 years ago

Hi @jtomaszewski,

You use hideBodyOverflow: true when you want the library to set overflow:hidden on the <body> element for IOS devices. I don't see any drawbacks of using it, in my opinion this should be always set but I thought that an "opt-in" change would be less intrusive for other users

paulborm commented 3 years ago

Any chances this PR will be merged soon? Without the fix this library is basically not usable on iOS devices.

diachedelic commented 3 years ago

I am willing to hide the body's overflow by default on iOS, so long as someone can post a minimal example showing where it's needed. I am unable to reproduce any issues on the iOS simulator with the demo (can anybody else?)

jvitela commented 3 years ago

Hi @diachedelic, I am able to reproduce on iOS (iPhone 11) by zooming in vertical layout and always on horizontal layout. Here is a video:

https://user-images.githubusercontent.com/839717/103633492-12ffef80-4f46-11eb-9494-0fa0d7ba9a32.mp4

joostvanhoof commented 3 years ago

What I found is that if you scroll on Safari, then stop (so there's no bottom toolbar), then you'll always be able to still scroll if you start your scroll in that bottom area where the toolbar was:

https://user-images.githubusercontent.com/5268113/103810945-63697100-505c-11eb-8bd1-9678d038b799.MOV

diachedelic commented 3 years ago

Yes, mobile safari has some weird behaviour near the toolbar, see https://benfrain.com/the-ios-safari-menu-bar-is-hostile-to-web-apps-discuss/. Does hiding the overflow prevent this then?

On 7 Jan 2021, at 6:19 am, Joost van Hoof notifications@github.com wrote:

What I found is that if you scroll on Safari, then stop (so there's no bottom toolbar), then you'll always be able to still scroll if you start your scroll in that bottom area where the toolbar was:

https://user-images.githubusercontent.com/5268113/103810945-63697100-505c-11eb-8bd1-9678d038b799.MOV https://user-images.githubusercontent.com/5268113/103810945-63697100-505c-11eb-8bd1-9678d038b799.MOV — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/willmcpo/body-scroll-lock/pull/207#issuecomment-755548538, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLZQGA47HADE2AAZSMEO3SYSZVNANCNFSM4UNP3M3Q.

joostvanhoof commented 3 years ago

Not sure what you mean by hiding the overflow?

What I’ve done is I’ve added a window.onscroll function forcing the scroll back to the original position on top of this library. Not very elegant but it does the job.

Op 7 jan. 2021 om 07:38 heeft diachedelic notifications@github.com het volgende geschreven:

Yes, mobile safari has some weird behaviour near the toolbar, see https://benfrain.com/the-ios-safari-menu-bar-is-hostile-to-web-apps-discuss/. Does hiding the overflow prevent this then?

On 7 Jan 2021, at 6:19 am, Joost van Hoof notifications@github.com wrote:

What I found is that if you scroll on Safari, then stop (so there's no bottom toolbar), then you'll always be able to still scroll if you start your scroll in that bottom area where the toolbar was:

https://user-images.githubusercontent.com/5268113/103810945-63697100-505c-11eb-8bd1-9678d038b799.MOV https://user-images.githubusercontent.com/5268113/103810945-63697100-505c-11eb-8bd1-9678d038b799.MOV — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/willmcpo/body-scroll-lock/pull/207#issuecomment-755548538, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLZQGA47HADE2AAZSMEO3SYSZVNANCNFSM4UNP3M3Q.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

diachedelic commented 3 years ago

This pull request is for setting overflow: hidden on iOS.

jvitela commented 3 years ago

@diachedelic In my case, setting overflow:hidden fixes the problem on iOS

joostvanhoof commented 3 years ago

I've tried this library which adds overflow: hidden to the body, but that doesn't solve the issue for me. I can still start scrolling from the bottom part where the toolbar normally is.

serhanguney commented 3 years ago

Same issue here with me. I am able to scroll from the bottom part of the screen. The weird thing is this issue happens randomly. Sometimes I'm not able to scroll, and sometimes I am. Does hideBodyOverflow() fix the issue ?

jvitela commented 3 years ago

Hi, @joostvanhoof and @yuivae,

This discussion thread is about a PR to set overflow: hidden to the body, in addition to other things this library already does to prevent the scroll. Please test this branch and tell us if it solved the problem for you too.

@joostvanhoof the demo at https://bodyscrolllock.vercel.app/ does not include the changes from this PR. The library you tested (https://github.com/FL3NKEY/scroll-lock) is not comparable, body-scroll-lock uses different strategies to prevent the scroll, so please test this PR instead.

Thanks you all for your help.

ryanbadger commented 3 years ago

Been having problems with this on iOS for a while, seems like overflow:hidden is supported now, so can this just be merged in so the npm is updated?

Looks like iOS has been updated and this npm doesn't work as well anymore, if you scroll to the bottom of a modal it locks up and you cannot scroll at all.

I was also seeing this quite a lot: Attempted to assign to readonly property.

I tested with overflow:hidden on body and works fine on iOS, so this pull request seems like a good addition to me.

willmcpo commented 3 years ago

Can we modify this PR to set the overflow:hidden on body for IOS by default? @jvitela

jvitela commented 3 years ago

Can we modify this PR to set the overflow:hidden on body for IOS by default? @jvitela

Yes, I will make a change to this PR.

gregorybolkenstijn commented 3 years ago

Any update on this? We're running into this issue and could really use this feature to solve some bugs in mobile Safari.

jvitela commented 3 years ago

Hi @willmcpo and @gregorybolkenstijn

Setting overflow:hidden for the body element solves the issue just for the modal example, after checking the vanilla example I found that I was still able to scroll with two fingers (iPhone 11)

After several tries and errors I found setting position:fixed for the body was the best solution.

Please try it and let me know if you are Ok with this solution.

babiellgr commented 3 years ago

Hi @willmcpo,

we are also interested in this PR, is there is chance this gets merged?

Thanks!

willmcpo commented 3 years ago

👀 Will have a review then merge.

willmcpo commented 3 years ago

Published v4.0.0-beta.0

Thanks for the efforts and the patience 🙏

KasperAndersson commented 3 years ago

Published v4.0.0-beta.0

Thanks for the efforts and the patience 🙏

Nice one, when is this planned to be released?

narnat commented 3 years ago

Hello @willmcpo, I'm using the last beta, it is working fine. The only issue is when I open a modal on a Safari mobile, the body position is resetting to the top, but the position is restored back to where it was after closing the modal. Currently, I'm using

 document.body.style.top = `-${window.scrollY}px`;

before opening a modal to not move the background (body). I would appreciate if there is gonna be a fix for this behavior.

willmcpo commented 3 years ago

@jvitela are you able to comment on that?

jvitela commented 3 years ago

Hi @narnat,

The current body position is already being saved before locking the scroll and later restored after releasing the scroll, see:

https://github.com/willmcpo/body-scroll-lock/pull/207/commits/cca8cda71f6bc1a00e52049628c84062fb269741#diff-643a4a126c8da9aa30956ce7bd9a85d41b9703c0b1ae59bf6c24b496afee10beR113

https://github.com/willmcpo/body-scroll-lock/pull/207/commits/cca8cda71f6bc1a00e52049628c84062fb269741#diff-643a4a126c8da9aa30956ce7bd9a85d41b9703c0b1ae59bf6c24b496afee10beR143

If this is not working in your case or not working as expected, can you please provide a minimum example to reproduce it? And also please specify the device you used for testing.

BwamNL commented 3 years ago

@jvitela I have also noticed this bug in the new beta. It is happening on both Safari and Chrome on my iPhone 12 Pro with iOS 14.6.

jvitela commented 3 years ago

Hi @BwamNL can you please provide a minimum example to reproduce it? I both pages from the examples/ folder work fine for me.

BwamNL commented 3 years ago

@jvitela I have a demo site where it happens: https://sb.flywheelstaging.com/ (u/p: studiobrabo / studiobrabo ) In the examples the scrolling works correct on my iPhone.

BwamNL commented 3 years ago

@jvitela I debugged a little bit, and i see the problems comes from not adding 'px'. I changed: document.body.style.top = -scrollY; to: document.body.style.top = '-' + scrollY + 'px'; and then the script works more or less. It still snaps weird when the bottomBar appears.

jvitela commented 3 years ago

It still snaps weird when the bottomBar appears.

Yes, I was also testing this yesterday. The snap is caused when the content area shrinks due to the bottom-bar AND address-bar appearing.

This extra code is used to detect if the content area changed and try to adjust the position: https://github.com/willmcpo/body-scroll-lock/commit/cca8cda71f6bc1a00e52049628c84062fb269741#diff-643a4a126c8da9aa30956ce7bd9a85d41b9703c0b1ae59bf6c24b496afee10beR125

The intention was to prevent the controls from being hidden if the content shrunk as it would happen in the basic example

IMG_67E92AE3D232-1

I think the delay is actually not necessary, since the timeout + animationFrame would ensure this is executed in a separate render cycle. Furthermore you can reduce the snap by adjusting to half the height difference (assuming the address and bottom bar take the same height amount)

    setTimeout(() => window.requestAnimationFrame(() => {
      // Attempt to check if the bottom AND address bar appeared due to the position change
      const contentHeight = innerHeight - window.innerHeight;
      if (contentHeight && scrollY >= innerHeight) {
        // Move the content further up so that the bottom bar doesn't hide it
        document.body.style.top = `-${scrollY + contentHeight * 0.5}px`;
      }
    }))

We can also discuss if this last part is actually needed or if we should get rid of it, try removing it let us know how it looks.

BwamNL commented 3 years ago

@jvitela I commented out everything for iOS (so no body.position fixed), and in our situation it works great without. The bottombar won't snap in, and we don't have problems with scrolling in the back of the modal. (You can check it here https://sb.flywheelstaging.com/ (u/p: studiobrabo / studiobrabo )

Maybe it should be optional for iOS?

jvitela commented 3 years ago

Hi @BwamNL, as I commented before, without position: fixed it is always possible to scroll by using two fingers and also short after you zoom:

https://user-images.githubusercontent.com/839717/124730245-75e03c80-df11-11eb-8fa1-afd67c7ccb1e.mov

kilianso commented 3 years ago

@jvitela It doesn't seem to work with version 4.0.0.beta.0 It always jumps to the top on iOS 14.6

Here's a demo using the latest beta: https://dkrut.csb.app/

And that's the corresponding Codesandbox: https://codesandbox.io/s/ios-body-scroll-lock-dkrut

If you downgrade the dependency to 3.1.5 it works like expected.

BwamNL commented 3 years ago

@jvitela you are right; with two fingers scrolling, it doesn't work. So we are back with the snapping ;)

simonmaass commented 3 years ago

for me this does lock the background but i am also not able to scroll in the modal

BwamNL commented 3 years ago

@jvitela will the fix be included in the 4.0 version?

jvitela commented 3 years ago

Hi @BwamNL I am not a maintainer of this library so I can't answer that. For this pull requests there are still a few issues based in your comments that I am trying to clear (although I currently don't have the time for it):

There seems to be a problem where the scroll position is reset, I believe this can be caused when the code is uglified as it was the case with your demo.
The snapping in the position when the browser address and bottom bars are displayed after the scroll is locked.

dkornilove commented 3 years ago

@jvitela @BwamNL here https://github.com/willmcpo/body-scroll-lock/blob/157d235d7c09350f92632ead7a60aacc3c88cd93/src/bodyScrollLock.js#L122 must be the string ending al least with "px", because browsers dont understand what units you want when passing a number to this style prop

jvitela commented 3 years ago

Hi @dkornilove , @BwamNL, @kilianso and @willmcpo

I created a new Pull-request which will hopefully fix the issues reported here: https://github.com/willmcpo/body-scroll-lock/pull/229

Please check it out and let me know if it works for you.

meendoo commented 2 years ago

I spotted this weird behavior: it seems there is a bug when you initiate 'touchmove' scroll by going upwards, it doesn't move. If you start scrolling downwards then revert direction it moves upwards fine.

jvitela commented 2 years ago

@meendoo did you test with the latest from this branch (https://github.com/jvitela/body-scroll-lock/tree/patch-1)? and can we move the conversation here https://github.com/willmcpo/body-scroll-lock/pull/229 ?

If the problem is still reproducible can you please upload a video with an example?

willmcpo / body-scroll-lock

Fix scroll lock for iOS devices #207