Skip to content

Commit a14451d

Browse files
deilvJohann-S
deilv
authored and
Johann-S
committed
Fix #18373: properly adjust padding-right of body and fixed elements when opening or closing modal
1 parent 91b6294 commit a14451d

File tree

2 files changed

+138
-39
lines changed

2 files changed

+138
-39
lines changed

‎js/src/modal.js

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ const Modal = (($) => {
6767
DIALOG : '.modal-dialog',
6868
DATA_TOGGLE : '[data-toggle="modal"]',
6969
DATA_DISMISS : '[data-dismiss="modal"]',
70-
FIXED_CONTENT : '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top'
70+
FIXED_CONTENT : '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top',
71+
NAVBAR_TOGGLER : '.navbar-toggler'
7172
}
7273

7374

@@ -212,7 +213,6 @@ const Modal = (($) => {
212213
this._isShown = null
213214
this._isBodyOverflowing = null
214215
this._ignoreBackdropClick = null
215-
this._originalBodyPadding = null
216216
this._scrollbarWidth = null
217217
}
218218

@@ -429,21 +429,53 @@ const Modal = (($) => {
429429
}
430430

431431
_setScrollbar() {
432-
const bodyPadding = parseInt(
433-
$(Selector.FIXED_CONTENT).css('padding-right') || 0,
434-
10
435-
)
432+
if (this._isBodyOverflowing) {
433+
// Note: DOMNode.style.paddingRight returns the actual value or '' if not set
434+
// while $(DOMNode).css('padding-right') returns the calculated value or 0 if not set
435+
436+
// Adjust fixed content padding
437+
$(Selector.FIXED_CONTENT).each((index, element) => {
438+
const actualPadding = $(element)[0].style.paddingRight
439+
const calculatedPadding = $(element).css('padding-right')
440+
$(element).data('padding-right', actualPadding).css('padding-right', `${parseFloat(calculatedPadding) + this._scrollbarWidth}px`)
441+
})
436442

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

439-
if (this._isBodyOverflowing) {
440-
document.body.style.paddingRight =
441-
`${bodyPadding + this._scrollbarWidth}px`
450+
// Adjust body padding
451+
const actualPadding = document.body.style.paddingRight
452+
const calculatedPadding = $('body').css('padding-right')
453+
$('body').data('padding-right', actualPadding).css('padding-right', `${parseFloat(calculatedPadding) + this._scrollbarWidth}px`)
442454
}
443455
}
444456

445457
_resetScrollbar() {
446-
document.body.style.paddingRight = this._originalBodyPadding
458+
// Restore fixed content padding
459+
$(Selector.FIXED_CONTENT).each((index, element) => {
460+
const padding = $(element).data('padding-right')
461+
if (typeof padding !== 'undefined') {
462+
$(element).css('padding-right', padding).removeData('padding-right')
463+
}
464+
})
465+
466+
// Restore navbar-toggler margin
467+
$(Selector.NAVBAR_TOGGLER).each((index, element) => {
468+
const margin = $(element).data('margin-right')
469+
if (typeof margin !== 'undefined') {
470+
$(element).css('margin-right', margin).removeData('margin-right')
471+
}
472+
})
473+
474+
// Restore body padding
475+
const padding = $('body').data('padding-right')
476+
if (typeof padding !== 'undefined') {
477+
$('body').css('padding-right', padding).removeData('padding-right')
478+
}
447479
}
448480

449481
_getScrollbarWidth() { // thx d.walsh

‎js/tests/unit/modal.js

Lines changed: 95 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ $(function () {
99
})
1010

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

339-
QUnit.test('should restore inline body padding after closing', function (assert) {
343+
QUnit.test('should adjust the inline body padding when opening and restore when closing', function (assert) {
340344
assert.expect(2)
341345
var done = assert.async()
342-
var originalBodyPad = 0
343346
var $body = $(document.body)
344-
345-
$body.css('padding-right', originalBodyPad)
347+
var originalPadding = $body.css('padding-right')
346348

347349
$('<div id="modal-test"/>')
348350
.on('hidden.bs.modal', function () {
349-
var currentBodyPad = parseInt($body.css('padding-right'), 10)
350-
assert.notStrictEqual($body.attr('style'), '', 'body has non-empty style attribute')
351-
assert.strictEqual(currentBodyPad, originalBodyPad, 'original body padding was not changed')
351+
var currentPadding = $body.css('padding-right')
352+
assert.strictEqual(currentPadding, originalPadding, 'body padding should be reset after closing')
352353
$body.removeAttr('style')
353354
done()
354355
})
355356
.on('shown.bs.modal', function () {
357+
var currentPadding = $body.css('padding-right')
358+
assert.notStrictEqual(currentPadding, originalPadding, 'body padding should be adjusted while opening')
356359
$(this).bootstrapModal('hide')
357360
})
358361
.bootstrapModal('show')
359362
})
360363

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

367371
$('<div id="modal-test"/>')
368372
.on('hidden.bs.modal', function () {
369-
assert.ok(!$body.attr('style'), 'body does not have inline padding set')
370-
$style.remove()
373+
assert.strictEqual($body.data('padding-right'), undefined, 'data-padding-right should be cleared after closing')
374+
$body.removeAttr('style')
371375
done()
372376
})
373377
.on('shown.bs.modal', function () {
378+
assert.strictEqual($body.data('padding-right'), originalPadding, 'original body padding should be stored in data-padding-right')
374379
$(this).bootstrapModal('hide')
375380
})
376381
.bootstrapModal('show')
377382
})
378383

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

385390
$('<div id="modal-test"/>')
391+
.on('hidden.bs.modal', function () {
392+
var currentPadding = $element.css('padding-right')
393+
assert.strictEqual(currentPadding, originalPadding, 'fixed element padding should be reset after closing')
394+
$element.remove()
395+
done()
396+
})
386397
.on('shown.bs.modal', function () {
387-
var paddingRight = parseInt($(document.body).css('padding-right'), 10)
388-
assert.strictEqual(isNaN(paddingRight), false)
389-
assert.strictEqual(paddingRight !== 0, true)
390-
$(document.body).css('padding-right', '') // Because test case "should ignore other inline styles when trying to restore body padding after closing" fail if not
398+
var currentPadding = $element.css('padding-right')
399+
assert.notStrictEqual(currentPadding, originalPadding, 'fixed element padding should be adjusted while opening')
400+
$(this).bootstrapModal('hide')
401+
})
402+
.bootstrapModal('show')
403+
})
404+
405+
QUnit.test('should store the original padding of fixed elements in data-padding-right before showing', function (assert) {
406+
assert.expect(2)
407+
var done = assert.async()
408+
var $element = $('<div class="fixed-top"></div>').appendTo('#qunit-fixture')
409+
var originalPadding = '0px'
410+
$element.css('padding-right', originalPadding)
411+
412+
$('<div id="modal-test"/>')
413+
.on('hidden.bs.modal', function () {
414+
assert.strictEqual($element.data('padding-right'), undefined, 'data-padding-right should be cleared after closing')
415+
$element.remove()
391416
done()
392417
})
418+
.on('shown.bs.modal', function () {
419+
assert.strictEqual($element.data('padding-right'), originalPadding, 'original fixed element padding should be stored in data-padding-right')
420+
$(this).bootstrapModal('hide')
421+
})
393422
.bootstrapModal('show')
394423
})
395424

396-
QUnit.test('should remove padding-right on modal after closing', function (assert) {
397-
assert.expect(3)
425+
QUnit.test('should adjust the inline margin of the navbar-toggler when opening and restore when closing', function (assert) {
426+
assert.expect(2)
427+
var done = assert.async()
428+
var $element = $('<div class="navbar-toggler"></div>').appendTo('#qunit-fixture')
429+
var originalMargin = $element.css('margin-right')
430+
431+
$('<div id="modal-test"/>')
432+
.on('hidden.bs.modal', function () {
433+
var currentMargin = $element.css('margin-right')
434+
assert.strictEqual(currentMargin, originalMargin, 'navbar-toggler margin should be reset after closing')
435+
$element.remove()
436+
done()
437+
})
438+
.on('shown.bs.modal', function () {
439+
var currentMargin = $element.css('margin-right')
440+
assert.notStrictEqual(currentMargin, originalMargin, 'navbar-toggler margin should be adjusted while opening')
441+
$(this).bootstrapModal('hide')
442+
})
443+
.bootstrapModal('show')
444+
})
445+
446+
QUnit.test('should store the original margin of the navbar-toggler in data-margin-right before showing', function (assert) {
447+
assert.expect(2)
398448
var done = assert.async()
399-
$('<div class="fixed-top fixed-bottom is-fixed sticky-top">@Johann-S</div>').appendTo('#qunit-fixture')
400-
$('.fixed-top, .fixed-bottom, .is-fixed, .sticky-top').css('padding-right', '10px')
449+
var $element = $('<div class="navbar-toggler"></div>').appendTo('#qunit-fixture')
450+
var originalMargin = '0px'
451+
$element.css('margin-right', originalMargin)
401452

402453
$('<div id="modal-test"/>')
454+
.on('hidden.bs.modal', function () {
455+
assert.strictEqual($element.data('margin-right'), undefined, 'data-margin-right should be cleared after closing')
456+
$element.remove()
457+
done()
458+
})
403459
.on('shown.bs.modal', function () {
404-
var paddingRight = parseInt($(document.body).css('padding-right'), 10)
405-
assert.strictEqual(isNaN(paddingRight), false)
406-
assert.strictEqual(paddingRight !== 0, true)
460+
assert.strictEqual($element.data('margin-right'), originalMargin, 'original navbar-toggler margin should be stored in data-margin-right')
407461
$(this).bootstrapModal('hide')
408462
})
463+
.bootstrapModal('show')
464+
})
465+
466+
QUnit.test('should ignore values set via CSS when trying to restore body padding after closing', function (assert) {
467+
assert.expect(1)
468+
var done = assert.async()
469+
var $body = $(document.body)
470+
var $style = $('<style>body { padding-right: 42px; }</style>').appendTo('head')
471+
472+
$('<div id="modal-test"/>')
409473
.on('hidden.bs.modal', function () {
410-
var paddingRight = parseInt($(document.body).css('padding-right'), 10)
411-
assert.strictEqual(paddingRight, 0)
474+
assert.ok(!$body.attr('style'), 'body does not have inline padding set')
475+
$style.remove()
412476
done()
413477
})
478+
.on('shown.bs.modal', function () {
479+
$(this).bootstrapModal('hide')
480+
})
414481
.bootstrapModal('show')
415482
})
416483

0 commit comments

Comments
 (0)