Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rewrite Modals.setScrollbar() and Modals.resetScrollbar() to properly handle padding-right of body and fixed elements #18441

Merged
merged 2 commits into from
Apr 2, 2017
Merged

Rewrite Modals.setScrollbar() and Modals.resetScrollbar() to properly handle padding-right of body and fixed elements #18441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08dc126
Fix #18373: properly adjust padding-right of body and fixed elements …
Mar 11, 2017
4b48847
Merge branch 'v4-dev' into fix-18373
deilv Apr 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 43 additions & 11 deletions js/src/modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ const Modal = (($) => {
DIALOG : '.modal-dialog',
DATA_TOGGLE : '[data-toggle="modal"]',
DATA_DISMISS : '[data-dismiss="modal"]',
FIXED_CONTENT : '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top'
FIXED_CONTENT : '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top',
NAVBAR_TOGGLER : '.navbar-toggler'
}


Expand Down Expand Up @@ -212,7 +213,6 @@ const Modal = (($) => {
this._isShown = null
this._isBodyOverflowing = null
this._ignoreBackdropClick = null
this._originalBodyPadding = null
this._scrollbarWidth = null
}

Expand Down Expand Up @@ -429,21 +429,53 @@ const Modal = (($) => {
}

_setScrollbar() {
const bodyPadding = parseInt(
$(Selector.FIXED_CONTENT).css('padding-right') || 0,
10
)
if (this._isBodyOverflowing) {
// Note: DOMNode.style.paddingRight returns the actual value or '' if not set
// while $(DOMNode).css('padding-right') returns the calculated value or 0 if not set

// Adjust fixed content padding
$(Selector.FIXED_CONTENT).each((index, element) => {
const actualPadding = $(element)[0].style.paddingRight
const calculatedPadding = $(element).css('padding-right')
$(element).data('padding-right', actualPadding).css('padding-right', `${parseFloat(calculatedPadding) + this._scrollbarWidth}px`)
})

this._originalBodyPadding = document.body.style.paddingRight || ''
// Adjust navbar-toggler margin
$(Selector.NAVBAR_TOGGLER).each((index, element) => {
const actualMargin = $(element)[0].style.marginRight
const calculatedMargin = $(element).css('margin-right')
$(element).data('margin-right', actualMargin).css('margin-right', `${parseFloat(calculatedMargin) + this._scrollbarWidth}px`)
})

if (this._isBodyOverflowing) {
document.body.style.paddingRight =
`${bodyPadding + this._scrollbarWidth}px`
// Adjust body padding
const actualPadding = document.body.style.paddingRight
const calculatedPadding = $('body').css('padding-right')
$('body').data('padding-right', actualPadding).css('padding-right', `${parseFloat(calculatedPadding) + this._scrollbarWidth}px`)
}
}

_resetScrollbar() {
document.body.style.paddingRight = this._originalBodyPadding
// Restore fixed content padding
$(Selector.FIXED_CONTENT).each((index, element) => {
const padding = $(element).data('padding-right')
if (typeof padding !== 'undefined') {
$(element).css('padding-right', padding).removeData('padding-right')
}
})

// Restore navbar-toggler margin
$(Selector.NAVBAR_TOGGLER).each((index, element) => {
const margin = $(element).data('margin-right')
if (typeof margin !== 'undefined') {
$(element).css('margin-right', margin).removeData('margin-right')
}
})

// Restore body padding
const padding = $('body').data('padding-right')
if (typeof padding !== 'undefined') {
$('body').css('padding-right', padding).removeData('padding-right')
}
}

_getScrollbarWidth() { // thx d.walsh
Expand Down
123 changes: 95 additions & 28 deletions js/tests/unit/modal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ $(function () {
})

QUnit.module('modal', {
before: function () {
// Enable the scrollbar measurer
$('<style type="text/css"> .modal-scrollbar-measure { position: absolute; top: -9999px; width: 50px; height: 50px; overflow: scroll; } </style>').appendTo('head')
},
beforeEach: function () {
// Run all tests in noConflict mode -- it's the only way to ensure that the plugin works in noConflict mode
$.fn.bootstrapModal = $.fn.modal.noConflict()
Expand Down Expand Up @@ -336,81 +340,144 @@ $(function () {
$toggleBtn.trigger('click')
})

QUnit.test('should restore inline body padding after closing', function (assert) {
QUnit.test('should adjust the inline body padding when opening and restore when closing', function (assert) {
assert.expect(2)
var done = assert.async()
var originalBodyPad = 0
var $body = $(document.body)

$body.css('padding-right', originalBodyPad)
var originalPadding = $body.css('padding-right')

$('<div id="modal-test"/>')
.on('hidden.bs.modal', function () {
var currentBodyPad = parseInt($body.css('padding-right'), 10)
assert.notStrictEqual($body.attr('style'), '', 'body has non-empty style attribute')
assert.strictEqual(currentBodyPad, originalBodyPad, 'original body padding was not changed')
var currentPadding = $body.css('padding-right')
assert.strictEqual(currentPadding, originalPadding, 'body padding should be reset after closing')
$body.removeAttr('style')
done()
})
.on('shown.bs.modal', function () {
var currentPadding = $body.css('padding-right')
assert.notStrictEqual(currentPadding, originalPadding, 'body padding should be adjusted while opening')
$(this).bootstrapModal('hide')
})
.bootstrapModal('show')
})

QUnit.test('should ignore values set via CSS when trying to restore body padding after closing', function (assert) {
assert.expect(1)
QUnit.test('should store the original body padding in data-padding-right before showing', function (assert) {
assert.expect(2)
var done = assert.async()
var $body = $(document.body)
var $style = $('<style>body { padding-right: 42px; }</style>').appendTo('head')
var originalPadding = '0px'
$body.css('padding-right', originalPadding)

$('<div id="modal-test"/>')
.on('hidden.bs.modal', function () {
assert.ok(!$body.attr('style'), 'body does not have inline padding set')
$style.remove()
assert.strictEqual($body.data('padding-right'), undefined, 'data-padding-right should be cleared after closing')
$body.removeAttr('style')
done()
})
.on('shown.bs.modal', function () {
assert.strictEqual($body.data('padding-right'), originalPadding, 'original body padding should be stored in data-padding-right')
$(this).bootstrapModal('hide')
})
.bootstrapModal('show')
})

QUnit.test('should have a paddingRight when the modal is taller than the viewport', function (assert) {
QUnit.test('should adjust the inline padding of fixed elements when opening and restore when closing', function (assert) {
assert.expect(2)
var done = assert.async()
$('<div class="fixed-top fixed-bottom sticky-top is-fixed">@Johann-S</div>').appendTo('#qunit-fixture')
$('.fixed-top, .fixed-bottom, .is-fixed, .sticky-top').css('padding-right', '10px')
var $element = $('<div class="fixed-top"></div>').appendTo('#qunit-fixture')
var originalPadding = $element.css('padding-right')

$('<div id="modal-test"/>')
.on('hidden.bs.modal', function () {
var currentPadding = $element.css('padding-right')
assert.strictEqual(currentPadding, originalPadding, 'fixed element padding should be reset after closing')
$element.remove()
done()
})
.on('shown.bs.modal', function () {
var paddingRight = parseInt($(document.body).css('padding-right'), 10)
assert.strictEqual(isNaN(paddingRight), false)
assert.strictEqual(paddingRight !== 0, true)
$(document.body).css('padding-right', '') // Because test case "should ignore other inline styles when trying to restore body padding after closing" fail if not
var currentPadding = $element.css('padding-right')
assert.notStrictEqual(currentPadding, originalPadding, 'fixed element padding should be adjusted while opening')
$(this).bootstrapModal('hide')
})
.bootstrapModal('show')
})

QUnit.test('should store the original padding of fixed elements in data-padding-right before showing', function (assert) {
assert.expect(2)
var done = assert.async()
var $element = $('<div class="fixed-top"></div>').appendTo('#qunit-fixture')
var originalPadding = '0px'
$element.css('padding-right', originalPadding)

$('<div id="modal-test"/>')
.on('hidden.bs.modal', function () {
assert.strictEqual($element.data('padding-right'), undefined, 'data-padding-right should be cleared after closing')
$element.remove()
done()
})
.on('shown.bs.modal', function () {
assert.strictEqual($element.data('padding-right'), originalPadding, 'original fixed element padding should be stored in data-padding-right')
$(this).bootstrapModal('hide')
})
.bootstrapModal('show')
})

QUnit.test('should remove padding-right on modal after closing', function (assert) {
assert.expect(3)
QUnit.test('should adjust the inline margin of the navbar-toggler when opening and restore when closing', function (assert) {
assert.expect(2)
var done = assert.async()
var $element = $('<div class="navbar-toggler"></div>').appendTo('#qunit-fixture')
var originalMargin = $element.css('margin-right')

$('<div id="modal-test"/>')
.on('hidden.bs.modal', function () {
var currentMargin = $element.css('margin-right')
assert.strictEqual(currentMargin, originalMargin, 'navbar-toggler margin should be reset after closing')
$element.remove()
done()
})
.on('shown.bs.modal', function () {
var currentMargin = $element.css('margin-right')
assert.notStrictEqual(currentMargin, originalMargin, 'navbar-toggler margin should be adjusted while opening')
$(this).bootstrapModal('hide')
})
.bootstrapModal('show')
})

QUnit.test('should store the original margin of the navbar-toggler in data-margin-right before showing', function (assert) {
assert.expect(2)
var done = assert.async()
$('<div class="fixed-top fixed-bottom is-fixed sticky-top">@Johann-S</div>').appendTo('#qunit-fixture')
$('.fixed-top, .fixed-bottom, .is-fixed, .sticky-top').css('padding-right', '10px')
var $element = $('<div class="navbar-toggler"></div>').appendTo('#qunit-fixture')
var originalMargin = '0px'
$element.css('margin-right', originalMargin)

$('<div id="modal-test"/>')
.on('hidden.bs.modal', function () {
assert.strictEqual($element.data('margin-right'), undefined, 'data-margin-right should be cleared after closing')
$element.remove()
done()
})
.on('shown.bs.modal', function () {
var paddingRight = parseInt($(document.body).css('padding-right'), 10)
assert.strictEqual(isNaN(paddingRight), false)
assert.strictEqual(paddingRight !== 0, true)
assert.strictEqual($element.data('margin-right'), originalMargin, 'original navbar-toggler margin should be stored in data-margin-right')
$(this).bootstrapModal('hide')
})
.bootstrapModal('show')
})

QUnit.test('should ignore values set via CSS when trying to restore body padding after closing', function (assert) {
assert.expect(1)
var done = assert.async()
var $body = $(document.body)
var $style = $('<style>body { padding-right: 42px; }</style>').appendTo('head')

$('<div id="modal-test"/>')
.on('hidden.bs.modal', function () {
var paddingRight = parseInt($(document.body).css('padding-right'), 10)
assert.strictEqual(paddingRight, 0)
assert.ok(!$body.attr('style'), 'body does not have inline padding set')
$style.remove()
done()
})
.on('shown.bs.modal', function () {
$(this).bootstrapModal('hide')
})
.bootstrapModal('show')
})

Expand Down