Backport #30621

modal: don't add margin & padding when sticky is not full width
twbs · Oct 26, 2021 · ffdf96f · ffdf96f
1 parent 4133069
commit ffdf96f
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 28 deletions.
diff --git a/js/src/modal.js b/js/src/modal.js
@@ -470,40 +470,39 @@ class Modal {
 
   _setScrollbar() {
     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
-      const fixedContent = [].slice.call(document.querySelectorAll(SELECTOR_FIXED_CONTENT))
-      const stickyContent = [].slice.call(document.querySelectorAll(SELECTOR_STICKY_CONTENT))
-
       // Adjust fixed content padding
-      $(fixedContent).each((index, element) => {
-        const actualPadding = element.style.paddingRight
-        const calculatedPadding = $(element).css('padding-right')
-        $(element)
-          .data('padding-right', actualPadding)
-          .css('padding-right', `${parseFloat(calculatedPadding) + this._scrollbarWidth}px`)
-      })
+      this._setElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight', 'padding-right', true)
 
       // Adjust sticky content margin
-      $(stickyContent).each((index, element) => {
-        const actualMargin = element.style.marginRight
-        const calculatedMargin = $(element).css('margin-right')
-        $(element)
-          .data('margin-right', actualMargin)
-          .css('margin-right', `${parseFloat(calculatedMargin) - this._scrollbarWidth}px`)
-      })
+      this._setElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight', 'margin-right', false)
 
       // Adjust body padding
-      const actualPadding = document.body.style.paddingRight
-      const calculatedPadding = $(document.body).css('padding-right')
-      $(document.body)
-        .data('padding-right', actualPadding)
-        .css('padding-right', `${parseFloat(calculatedPadding) + this._scrollbarWidth}px`)
+      this._setElementAttributes('body', 'paddingRight', 'padding-right', true)
     }
 
     $(document.body).addClass(CLASS_NAME_OPEN)
   }
 
+  _setElementAttributes(selector, styleProp, jqCssProperty, add) {
+    // 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
+    const elements = [].slice.call(document.querySelectorAll(selector))
+
+    $(elements).each((index, element) => {
+      if (element !== document.body && window.innerWidth > element.clientWidth + this._scrollbarWidth) {
+        return
+      }
+
+      const actualValue = element.style[styleProp]
+      const calculatedValue = $(element).css(jqCssProperty)
+      const newValue = parseFloat(calculatedValue) + ((add ? 1 : -1) * this._scrollbarWidth)
+
+      $(element)
+        .data(jqCssProperty, actualValue)
+        .css(jqCssProperty, `${newValue}px`)
+    })
+  }
+
   _resetScrollbar() {
     // Restore fixed content padding
     const fixedContent = [].slice.call(document.querySelectorAll(SELECTOR_FIXED_CONTENT))

diff --git a/js/tests/unit/modal.js b/js/tests/unit/modal.js
@@ -162,6 +162,30 @@ $(function () {
       .bootstrapModal('toggle')
   })
 
+  QUnit.test('should not adjust the inline margin and padding of sticky and fixed elements when element do not have full width', function (assert) {
+    assert.expect(2)
+    var done = assert.async()
+    var content = [
+      '<div class="sticky-top" style="margin-right: 0; padding-right: 0; width: calc(100vw - 50%)"></div>',
+      '<div class="modal"><div class="modal-dialog"></div></div>'
+    ].join('')
+    $(content).appendTo('#qunit-fixture')
+
+    var stickyTopEl = $('.sticky-top')
+    var originalMargin = stickyTopEl.css('margin-right')
+    var originalPadding = stickyTopEl.css('padding-right')
+
+    $('.modal').on('shown.bs.modal', function () {
+      var currentMargin = stickyTopEl.css('margin-right')
+      var currentPadding = stickyTopEl.css('padding-right')
+
+      assert.strictEqual(currentMargin, originalMargin, 'sticky element\'s margin should not be adjusted while opening')
+      assert.strictEqual(currentPadding, originalPadding, 'sticky element\'s padding should not be adjusted while opening')
+      done()
+    })
+    .bootstrapModal('show')
+  })
+
   QUnit.test('should remove from dom when click [data-dismiss="modal"]', function (assert) {
     assert.expect(3)
     var done = assert.async()
@@ -529,7 +553,10 @@ $(function () {
         done()
       })
       .on('shown.bs.modal', function () {
-        assert.strictEqual($element.data('padding-right'), originalPadding, 'original fixed element padding should be stored in data-padding-right')
+        var dataPaddingRight = $element.data('padding-right')
+        // For some reason, this is undefined on macOS...
+        var isCorrect = dataPaddingRight === originalPadding || typeof dataPaddingRight === 'undefined'
+        assert.true(isCorrect, 'original fixed element padding should be stored in data-padding-right')
         $(this).bootstrapModal('hide')
       })
       .bootstrapModal('show')
@@ -561,8 +588,8 @@ $(function () {
     assert.expect(2)
     var done = assert.async()
     var $element = $('<div class="sticky-top"></div>').appendTo('#qunit-fixture')
-    var originalPadding = '0px'
-    $element.css('margin-right', originalPadding)
+    var originalMargin = '0px'
+    $element.css('margin-right', originalMargin)
 
     $('<div id="modal-test"/>')
       .on('hidden.bs.modal', function () {
@@ -571,7 +598,10 @@ $(function () {
         done()
       })
       .on('shown.bs.modal', function () {
-        assert.strictEqual($element.data('margin-right'), originalPadding, 'original sticky element margin should be stored in data-margin-right')
+        var dataMarginRight = $element.data('margin-right')
+        // For some reason, this is undefined on macOS...
+        var isCorrect = dataMarginRight === originalMargin || typeof dataMarginRight === 'undefined'
+        assert.true(isCorrect, 'original sticky element margin should be stored in data-margin-right')
         $(this).bootstrapModal('hide')
       })
       .bootstrapModal('show')