willmcpo / body-scroll-lock

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

Using reserveScrollBarGap option with vanilla JS #195

Open martingarnett01 opened 3 years ago

martingarnett01 commented 3 years ago

Hello,

I'm using the vanilla JS implementation of this library, however, I'm having struggles with passing in the reserveScrollBarGap option.

The body scroll lock is working absolutely fine, but I can't work out how to pass in the reserveScrollBarGap option?

This is what I have:

bodyScrollLock.enableBodyScroll( lightbox, {reserveScrollBarGap: true} );

(Note: lightbox is my target element as per the docs - this all works fine)

Am I doing something wrong? Or does this option not work with the vanilla JS implementation?

Thanks in advance!

paulborm commented 3 years ago

Hi @martingarnett01, I think you have to use it on the disableBodyScroll() function enstead of enableBodyScroll(). Because you want the scrollbar to be reserved at the time you DISABLE the body scroll :-)

https://github.com/willmcpo/body-scroll-lock#reservescrollbargap

martingarnett01 commented 3 years ago

Doh! Thank-you @paulborm ! Feel free to revoke my developer license on this one.

One question though, now that it works...

There's a slight delay with adding the padding, so the content shifts right, then shifts back very quickly. Any idea/thoughts on this?

Here's a recording from my site: https://imgur.com/a/AD19P7k

doutatsu commented 3 years ago

@martingarnett01 This is due to your transition timing. The way to avoid it, is to just add a timeout. For example, this is what I do for my modal transitions:

        if (val) {
          // reserveScrollBarGap applies padding when disabling scrollbar
          disableBodyScroll(this.$refs.modal, { reserveScrollBarGap: true });
        } else {
          // Need to wait for animation to finish, to avoid content jump due to
          // padding change
          const el = this.targetElement;
          setTimeout(() => { enableBodyScroll(el); }, 200);
        }
martingarnett01 commented 3 years ago

@doutatsu Thanks for the reply. I tried your suggestion by adding a timeout to the enableBodyScroll function. Although it helps 'hide' the jank on the main page, it emphasises it on my modal:

https://imgur.com/a/5lEWfmT

It's almost like the bodyScrollLock function is first removing the scroll overflow, THEN it's adding the padding. Where it needs to do it both at the same time?

doutatsu commented 3 years ago

@martingarnett01 I've noticed that there is some weird behaviour in the latest version actually. I myself reverted back to 3.0.2 version, as latest version introduced a jump to the scroll bar for me as well. Not sure if it could be your case as well, but I recommend you rollback to that version and check if it helps

martingarnett01 commented 3 years ago

@doutatsu Hazah! That did the trick. Thank you so much.

As you can probably tell by my OP, I'm new to developing and so I have no idea what improvement to suggest to fix this. I've also never contributed to open source before (still looking for an entry-level project for me to work on, heh.)

Should I close this issue or leave it open for tracking purposes?

doutatsu commented 3 years ago

@martingarnett01 No worries glad it worked. I couldn't quite see what the latest changes did that broke it, I suspect #189, but couldn't track down the issue, hence didn't open an issue myself. I'll leave this open for now and tag @willmcpo or @diachedelic so maybe they could take a look