fix(modal): port @thcipriani's solution to modal body shift

[MrOrz]

This pull request addresses the scrolling problem that dates back to an old issue twbs/bootstrap#9855, which is also related to #879.

When the modal dialog opens, the content of body is shifted due to the absence of the vertical scrollbar.

Demonstration of the bug: http://plnkr.co/edit/aEaIphp0kGBC3G9dNw6j

The solution was proposed by @thcipriani in March this year, and is shipped with Bootstrap v3.2.0. This pull request contains a port of @thcipriani 's solution.

Demonstration of the solution: http://plnkr.co/edit/n1OQrpXPx7MTcozZrJA7

For Bootstrap v3.1.1, we need to add the following CSS for the solution to work.

.modal-scrollbar-measure {
  position: absolute;
  top: -9999px;
  width: 50px;
  height: 50px;
  overflow: scroll;
}

.modal-open .modal {
  overflow-x: hidden;
  overflow-y: auto;
}

[apitts]

@MrOrz A question / suggestion on this pull request (which I think is really useful as that body content shift is quite annoying). Think it may be worth changing the line:

if (bodyEl[0].clientWidth >= $window.innerWidth) { return; }

to:

if (bodyEl[0].clientWidth >= $window.innerWidth) { scrollbarWidth = 0; return; }

Otherwise, if the scrollbar is no longer there (eg. deleting items in a table) or has been added (e.g. adding items in a table) then the shift is reintroduced. You can simulate this quickly by increasing the height of developer tools in Chrome on a window with no scrollbar.

Also, perhaps worth deleting the console.log statements?

[MrOrz]

@apitts glad to hear from you!

The branch is now updated and the updated plunkr is here: http://plnkr.co/edit/aF8NdUSHnjjcjspo8gw7

Should I squash the commits?

[patrickmarabeas]

It would also be good if there was a generic class which also received the right hand padding - as fixed positioned elements (eg fixed header) aren't effected by padding added to the body...

Thanks!

[wesleycho]

Verified this is still an issue in this Plunker, even with current Bootstrap.

[wesleycho]

I created a build based off of current master with this patch, and this seems to have not fixed the problem, as can be seen by this Plunker. One big change that landed that may affect this is that the modal now has ngAnimate support, but this can be reproduced even without ngAnimate loaded.

Can you investigate what has changed that affects this implementation and reimplement this in a way that fixes the issue on current master?

[thcipriani]

@wesleycho fwiw, bootstrap recently had a regression that was fixed fairly nicely by @hnrch02 in https://github.com/twbs/bootstrap/pull/15378—didn't look to see what build you're using of bootstrap.

[wesleycho]

The build in the Plunker is just 3.3.4, although I tried with 3.1.1 I believe.

[chrisirhc]

Unfortunately, as mentioned in the commit that fixed this in twbs/bootstrap@0907244, it introduces a new bug: twbs/bootstrap#14040 . In light of that, I'm moving this out of 0.13.0 release for later consideration perhaps when a better solution is found.

[Klaster1]

In case someone else wants a fix, but not to use other repo/fork, I've made a dirty hack based on @MrOrz's pull request and bootstrap code: https://gist.github.com/Klaster1/b52311698c73e1d83b95. It uses MutationObserver on body, so you'll need polyfill for older browsers. Keep in mind that I didn't test it for all possible scenarios, so issues might happen.

[Foxandxss]

This is really old. If this is still an issue, I would love a new issue with an up to date plunker so we can fix it.

Thank you.