Skip to content

Commit

Permalink
Modal body shift fix for IE10/11
Browse files Browse the repository at this point in the history 
Closes #13103 by merging it.
  • Loading branch information
@thcipriani @cvrebert
thcipriani authored and cvrebert committed Mar 29, 2014
1 parent 28e6c34 commit 0907244
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 20 deletions.
23 changes: 14 additions & 9 deletions dist/js/bootstrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -795,11 +795,12 @@ if (typeof jQuery === 'undefined') { throw new Error('Bootstrap\'s JavaScript re
// ======================

var Modal = function (element, options) {
this.options = options
this.$body = $(document.body)
this.$element = $(element)
this.$backdrop =
this.isShown = null
this.options = options
this.$body = $(document.body)
this.$element = $(element)
this.$backdrop =
this.isShown = null
this.scrollbarWidth = 0

if (this.options.remote) {
this.$element
Expand Down Expand Up @@ -830,6 +831,7 @@ if (typeof jQuery === 'undefined') { throw new Error('Bootstrap\'s JavaScript re

this.isShown = true

this.checkScrollbar()
this.$body.addClass('modal-open')

this.setScrollbar()
Expand Down Expand Up @@ -976,11 +978,14 @@ if (typeof jQuery === 'undefined') { throw new Error('Bootstrap\'s JavaScript re
}
}

Modal.prototype.checkScrollbar = function () {
if (document.body.clientWidth >= window.innerWidth) return
this.scrollbarWidth = this.scrollbarWidth || this.measureScrollbar()
}

Modal.prototype.setScrollbar = function () {
if (document.body.clientHeight <= window.innerHeight) return
var scrollbarWidth = this.measureScrollbar()
var bodyPad = parseInt(this.$body.css('padding-right') || 0)
if (scrollbarWidth) this.$body.css('padding-right', bodyPad + scrollbarWidth)
var bodyPad = parseInt(this.$body.css('padding-right') || 0)
if (this.scrollbarWidth) this.$body.css('padding-right', bodyPad + this.scrollbarWidth)
}

Modal.prototype.resetScrollbar = function () {
Expand Down
2 changes: 1 addition & 1 deletion dist/js/bootstrap.min.js
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion docs/dist/js/bootstrap.min.js
View file

Large diffs are not rendered by default.

23 changes: 14 additions & 9 deletions js/modal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@
// ======================

var Modal = function (element, options) {
this.options = options
this.$body = $(document.body)
this.$element = $(element)
this.$backdrop =
this.isShown = null
this.options = options
this.$body = $(document.body)
this.$element = $(element)
this.$backdrop =
this.isShown = null
this.scrollbarWidth = 0

if (this.options.remote) {
this.$element
Expand Down Expand Up @@ -49,6 +50,7 @@

this.isShown = true

this.checkScrollbar()
this.$body.addClass('modal-open')

this.setScrollbar()
Expand Down Expand Up @@ -195,11 +197,14 @@
}
}

Modal.prototype.checkScrollbar = function () {
if (document.body.clientWidth >= window.innerWidth) return
this.scrollbarWidth = this.scrollbarWidth || this.measureScrollbar()
}

Modal.prototype.setScrollbar = function () {
if (document.body.clientHeight <= window.innerHeight) return
var scrollbarWidth = this.measureScrollbar()
var bodyPad = parseInt(this.$body.css('padding-right') || 0)
if (scrollbarWidth) this.$body.css('padding-right', bodyPad + scrollbarWidth)
var bodyPad = parseInt(this.$body.css('padding-right') || 0)
if (this.scrollbarWidth) this.$body.css('padding-right', bodyPad + this.scrollbarWidth)
}

Modal.prototype.resetScrollbar = function () {
Expand Down

7 comments on commit 0907244

@cvrebert
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supposedly really truly fixes #9855.

@andrejcremoznik
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could someone enlighten me how does this fix shifting of elements inside a .navbar-fixed-top element? Non fixed elements jumping and fixed elements staying the same is way worse than the whole page jumping. Why is hiding the scrollbar even necessary? If a page already displays a scrollbar, just keep it visible for the open modal.
#9855 is thus not fixed.

@cvrebert
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrejcremoznik See #14040.

@liu-xinhui
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on v3.3.2 this bug reappear

@liu-xinhui
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v3.3.0 is well,but v3.3.2 reappear on ie11,but chrome is well

@cvrebert
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liunewshine Kindly read the above comments and linked open issue before posting superfluous comments.

@anishpali1
Copy link

@anishpali1 anishpali1 commented on 0907244 Dec 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue persist with v3.3.6 in chrome 47.0.2526.80 and firefox 42

Please sign in to comment.