Hi @brendon,

thanks for your PR.
Can you provide a codepen which reproduces it and a unit-test which fails with the current master but succeeds with your change?

Hi @DanielRuf, I'll sort those changes for you. This will be my first (that I can recall) unit test in javascript. Would you be able to help by pointing me at an appropriate place in the current tests where I should put this test (if there's any existing testing around scroll prevention (I couldn't find any))?

If you're worried about it being a breaking change then we could approach it in a different manner where we make it an option that can be turned off (i.e. don't prevent scrolling). In my case the picker is showing up on a form that's already in a modal so all of that work has been done by the modal.

The third option could be to allow the element itself to be defined by the user (e.g. either the html tag or body etc...)

Let me know which way you think is best.

I've done some more testing to work out why the code is failing in my particular situation and that's because semantic-ui modal (that I use) sets overflow: hidden on the body tag already, and then setting overflow: hidden on html after this scrolls us to the top of the page.

Consensus seems to be to set it on the body tag and not the html tag, though I note one person mentions that this still allows scrolling for iOS though I haven't tested that.

I'll wait on your opinion of the best way to work around this and then get to work on the code. :)

Ok, I have a working codepen. You need to use Chrome to trigger the issue: https://codepen.io/brendon-the-animator/pen/pQKWrM

First scroll down, then click the button to set overflow: hidden on the body (simulating most modals), then click on the input to trigger the date box. You'll see the page scroll to the top.

Regarding a unit test, this is difficult because it's a browser-specific behaviour. I couldn't get it to reliably test out.

Regarding a unit test, this is difficult because it's a browser-specific behaviour

Isn't this a browserbug then? We should not fix bugs in browsers.

Shouldn't we do our best to avoid the bug though? Setting overflow: hidden on the body tag instead of the html tag achieves the same result but doesn't trigger the bug. I don't see a downside and my searching hasn't turned up any issues with setting overflow hidden on the body tag instead. iOS Safari ignores that style on both html and body so there's nothing to be done there.

If you look at Bootstrap's modal, you'll see they too set overflow: hidden on just the body tag:

https://getbootstrap.com/docs/4.0/components/modal/#live-demo

Let's try this and see if we can test it with a unit test (using scrollHeight, computedStyle and so on).

Thanks @DanielRuf, this is how far I got:

module( 'Overflow hidden', {
    setup: function() {
        $DOM.append( $INPUT.clone() )
        var $input = $DOM.find( 'input' ).pickadate()
        this.picker = $input.pickadate( 'picker' )
        $('html').css('height', '100%')
        $('body').css('height', '100%')
        $('body').css('overflow', 'hidden')
    },
    teardown: function() {
        $('body').css('overflow', '')
        $('html').css('height', '')
        $('body').css('height', '')
        this.picker.stop()
        $DOM.empty()
    }
})

test( 'Picker does not change the window scrollY', function() {

    var picker = this.picker

    window.scrollTo(0,500)

    picker.open()
    picker.close()
    strictEqual( window.scrollY, 500, 'Kept scrollY untouched' )
})

@brendon awesome, this looks great so far!

Can you open a new (WIP) PR for this? I'll do a review when it is ready to merge.

I'll check the docs then if we havd to update something or mention this change somewhere.

Hi @DanielRuf, would you mind doing a new release?

Hi @DanielRuf, sorry to bump this, but would it be possible to do a new release so I can stop using GitHub for my yarn source for this project? :)

Hi @brendon,

can you create a new issue for this? I would coordinate and plan the new release there.

But keep in mind that we are currently working on v5.

Thanks @DanielRuf, will do :)