Fix scroll lock for iOS devices #207

jvitela · 2020-12-04T14:16:11Z

As commented here: #200 (comment)
Other libraries successfully use this strategy on iOS devices, this could be an opt-in to solve many iOS related issues reported here.

As commented here: willmcpo#200 (comment) Other libraries successfully use this strategy on iOS devices, this could be an opt-in to solve many iOS related issues reported here.

src/bodyScrollLock.js

jtomaszewski · 2021-01-03T16:57:34Z

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 · 2021-01-04T10:01:03Z

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 · 2021-01-04T13:30:01Z

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

diachedelic · 2021-01-04T23:52:47Z

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 · 2021-01-05T10:07:04Z

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:

issue-520.mp4

joostvanhoof · 2021-01-06T19:19:34Z

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:

Image.from.iOS.MOV

diachedelic · 2021-01-07T06:38:03Z

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 ***@***.***> 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 <#207 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACLZQGA47HADE2AAZSMEO3SYSZVNANCNFSM4UNP3M3Q>.

joostvanhoof · 2021-01-07T07:01:15Z

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 ***@***.***> 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 ***@***.***> 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 <#207 (comment)>, 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 · 2021-01-07T07:12:59Z

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

jvitela · 2021-01-07T08:44:19Z

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

joostvanhoof · 2021-01-07T10:08:22Z

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 · 2021-01-14T12:46:35Z

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 · 2021-02-07T09:47:30Z

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 · 2021-02-13T12:50:35Z

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 · 2021-02-16T13:01:26Z

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

jvitela · 2021-02-18T16:40:28Z

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 · 2021-03-22T15:44:42Z

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

jvitela · 2021-04-08T15:52:18Z

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 · 2021-06-16T14:12:13Z

Hi @willmcpo,

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

Thanks!

willmcpo · 2021-06-16T14:27:46Z

👀
Will have a review then merge.

willmcpo · 2021-06-23T13:16:15Z

Published v4.0.0-beta.0

Thanks for the efforts and the patience 🙏

KasperAndersson · 2021-06-28T12:16:58Z

Published v4.0.0-beta.0

Thanks for the efforts and the patience 🙏

Nice one, when is this planned to be released?

narnat · 2021-06-28T20:38:58Z

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 · 2021-06-29T06:14:42Z

@jvitela are you able to comment on that?

jvitela · 2021-06-29T14:56:12Z

Hi @narnat,

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

cca8cda#diff-643a4a126c8da9aa30956ce7bd9a85d41b9703c0b1ae59bf6c24b496afee10beR113

cca8cda#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 · 2021-07-01T06:05:43Z

@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 · 2021-07-01T07:21:52Z

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

BwamNL · 2021-07-01T12:13:26Z

@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 · 2021-07-06T08:57:44Z

@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 · 2021-07-06T09:30:47Z

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: cca8cda#diff-643a4a126c8da9aa30956ce7bd9a85d41b9703c0b1ae59bf6c24b496afee10beR125

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

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 · 2021-07-07T07:53:37Z

@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 · 2021-07-07T08:54:43Z

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:

studiobrabo.mov

kilianso · 2021-07-07T09:18:01Z

@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 · 2021-07-07T09:24:33Z

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

simonmaass · 2021-07-23T14:17:59Z

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

BwamNL · 2021-07-28T11:24:55Z

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

jvitela · 2021-07-28T11:39:16Z

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 · 2021-08-09T17:11:51Z

@jvitela @BwamNL
here

body-scroll-lock/src/bodyScrollLock.js

Line 122 in 157d235

document.body.style.top = -scrollY;

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 · 2021-08-10T21:14:42Z

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

I created a new Pull-request which will hopefully fix the issues reported here: #229

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

Trendymen · 2021-08-13T13:17:58Z

src/bodyScrollLock.js

@@ -229,7 +272,9 @@ export const enableBodyScroll = (targetElement: any): void => {
    }
  }

-  if (isBodyOverflowHidden) {
+  if (isIosDevice) {
+    restorePositionSetting();


locks.length should be check there.

Maybe should return before this if lock.length !== 0

if (isIosDevice) { targetElement.ontouchstart = null; targetElement.ontouchmove = null; if (documentListenerAdded && locks.length === 0) { document.removeEventListener('touchmove', preventDefault, hasPassiveEvents ? { passive: false } : undefined); documentListenerAdded = false; } }

Trendymen

sorry,I opened the review,just sugggestions.

meendoo · 2021-09-02T10:38:28Z

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 · 2021-09-03T13:22:51Z

@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 #229 ?

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

Add option to set overflow:hidden on the body

599cce7

As commented here: willmcpo#200 (comment) Other libraries successfully use this strategy on iOS devices, this could be an opt-in to solve many iOS related issues reported here.

fabianludwig reviewed Dec 7, 2020

View reviewed changes

src/bodyScrollLock.js Outdated Show resolved Hide resolved

fix: typo in BodyScrollOptions interface

afc3b6a

This was referenced Dec 14, 2020

Body scroll is not blocked on IOs #206

Open

Doesn't work on iOS mobile devices #204

Open

jvitela added 3 commits April 8, 2021 17:46

refactor: set fixed position for body for iOS devices

cca8cda

chore: built library

2c8f0ba

chore: updated examples

3b7877b

jvitela changed the title ~~Add option to set overflow:hidden on the body~~ Fix scroll lock for iOS devices Apr 8, 2021

collegetransitions mentioned this pull request May 17, 2021

Modal content not scrolling on iOS Safari chakra-ui/chakra-ui-vue#432

Open

pomacs-store approved these changes Jun 23, 2021

View reviewed changes

willmcpo approved these changes Jun 23, 2021

View reviewed changes

willmcpo merged commit 3d22034 into willmcpo:master Jun 23, 2021

bhj mentioned this pull request Jun 25, 2021

Potential iOS scroll bug pradel/react-responsive-modal#468

Open

jvitela mentioned this pull request Aug 10, 2021

Fix bugs from previous patch #229

Open

Trendymen reviewed Aug 13, 2021

View reviewed changes

admench mentioned this pull request Apr 25, 2023

Stack modal does not scroll well on iOS device statamic/cms#7989

Closed

Fix scroll lock for iOS devices #207

Fix scroll lock for iOS devices #207

Conversation

jvitela commented Dec 4, 2020

jtomaszewski commented Jan 3, 2021

jvitela commented Jan 4, 2021

paulborm commented Jan 4, 2021 • edited

diachedelic commented Jan 4, 2021

jvitela commented Jan 5, 2021

joostvanhoof commented Jan 6, 2021

diachedelic commented Jan 7, 2021 via email

joostvanhoof commented Jan 7, 2021 via email

diachedelic commented Jan 7, 2021

jvitela commented Jan 7, 2021

joostvanhoof commented Jan 7, 2021

serhanguney commented Jan 14, 2021

jvitela commented Feb 7, 2021 • edited

ryanbadger commented Feb 13, 2021 • edited

willmcpo commented Feb 16, 2021 • edited

jvitela commented Feb 18, 2021

gregorybolkenstijn commented Mar 22, 2021

jvitela commented Apr 8, 2021

babiellgr commented Jun 16, 2021

willmcpo commented Jun 16, 2021

willmcpo commented Jun 23, 2021

KasperAndersson commented Jun 28, 2021

narnat commented Jun 28, 2021

willmcpo commented Jun 29, 2021

jvitela commented Jun 29, 2021

BwamNL commented Jul 1, 2021

jvitela commented Jul 1, 2021

BwamNL commented Jul 1, 2021

BwamNL commented Jul 6, 2021

jvitela commented Jul 6, 2021 • edited

BwamNL commented Jul 7, 2021

jvitela commented Jul 7, 2021 • edited

kilianso commented Jul 7, 2021 • edited

BwamNL commented Jul 7, 2021

simonmaass commented Jul 23, 2021

BwamNL commented Jul 28, 2021

jvitela commented Jul 28, 2021

dkornilove commented Aug 9, 2021

jvitela commented Aug 10, 2021

Trendymen Aug 13, 2021

Choose a reason for hiding this comment

Trendymen Aug 13, 2021

Choose a reason for hiding this comment

Trendymen left a comment

Choose a reason for hiding this comment

meendoo commented Sep 2, 2021 • edited

jvitela commented Sep 3, 2021

paulborm commented Jan 4, 2021 •

edited

jvitela commented Feb 7, 2021 •

edited

ryanbadger commented Feb 13, 2021 •

edited

willmcpo commented Feb 16, 2021 •

edited

jvitela commented Jul 6, 2021 •

edited

jvitela commented Jul 7, 2021 •

edited

kilianso commented Jul 7, 2021 •

edited

meendoo commented Sep 2, 2021 •

edited