Fix for broken scrolling on tall modals.

[RickKukiela]

Modified dom layout to insert the overlay div in the body instead of inside element root. Modified css to account for this change. Reason for change is that scrolling is absolutely broken on the existing structure so I changed it. You can now scroll properly when the vex modal is taller than the viewport.

[RickKukiela]

Obviously you can ignore the changes in the dist folder, I just re-ran grunt to ensure everything was good to go with my edits, so everything was recompiled in dist based off my edited source files.

[bbatliner]

I tried out your changes and the scrolling problem was fixed! I wasn't able to get the vex window to close when clicking on the overlay. The overlayClosesOnClick option should still work with these changes. Can you confirm that your changes prevent clicking on the overlay from closing the vex?

An alternative fix that would avoid a breaking change (DOM structure), to quote @adamschwartz:

The simple fix is to simply apply pointer-events: none to .vex-overlay. Now the mousewheel event isn't ever seen by .vex-overlay and instead is fired directly on .vex.

The current browser support for pointer-events looks good: https://caniuse.com/#search=pointer-events. Do you need to support <IE11 or Opera Mini? This would be one line of CSS vs a breaking structure change.

[RickKukiela]

Honestly I wouldn't care if ie<11 or opera mini didnt work right is like .1% of the web at this point. I dont think I tested the close event on click so i'll have to double check that but the way I integrated it should have worked. Let me take a look.

[RickKukiela]

OK I fixed my code, the problem was the overlay was behind rootEl and therefore could never be clicked so I just attached the onclick event to rootEl instead and now it appears to be working.

[RickKukiela]

@bbatliner So I know this is a breaking change, technically but unless someone is doing something really crazy with it, it should really have no effect for 99% of users, if not all users. That being said, if I had to pick between the two fixes I suppose I would go with mine since its actually backwards compatible with older IE versions, even if they are a very small market share, working for more people is always better, if its possible. In any event its up to you. I haven't tested the pointer-events thing but if you say it works I'll take your word for it.

[bbatliner]

Great, thanks for fixing that. I agree, your changes are backwards compatible and are preferred. Since it's technically a breaking change we'd have to bump the version to 4.0 on npm. Not a huge deal. I'll double check the PR soon and get it merged in.

Thanks!

[bbatliner]

@adamschwartz can we get an npm/bower/component release and a rebuild of the docs for 4.0.0? Breaking change here in the DOM structure bumps major version. I'll make the GitHub release now.

[adamschwartz]

Done.

For future reference though, @bbatliner I believe I made you an owner.

adam in ~/dev/projects/vex on master!
± npm owner ls
adamschwartz <...@gmail.com>
bbatliner <...@gmail.com>
geekjuice <...@gmail.com>

[bbatliner]

@SublymeRick have you seen #237? My hunch is this DOM change, to move the overlay, could be the culprit.

[RickKukiela]

Without looking more into it, I would assume so. Remember when I said "@bbatliner So I know this is a breaking change, technically but unless someone is doing something really crazy with it, it should really have no effect for 99% of users, if not all users."? Well the demo page with the stacking fanciness is what I would consider "doing something crazy with it". So there is that. Also you did say you were going to bump the version on this since it was, technically speaking, a breaking change. If the demo stack thing wasn't updated to accommodate this change then its likely it experience a problem such as this.

I would like to help out with this, but to be honest, the code for that stacking thing is little over my head, and I'm so busy with work right now I just literally don't have the free time to look at it :(