Add Tests on scrollbar.js & better handling if a style property doesn't exists (#33948)

* scrollbar.js:
add some tests
transfer test from modal.spec. to scrollbar.spec
proper handling if style property doesn't exist

1 files changed, 119 insertions, 36 deletions

diff --git a/js/tests/unit/util/scrollbar.spec.js b/js/tests/unit/util/scrollbar.spec.js
index e09a37ce7..b51c8a979 100644
--- a/js/tests/unit/util/scrollbar.spec.js
+++ b/js/tests/unit/util/scrollbar.spec.js
@@ -4,10 +4,13 @@ import Manipulator from '../../../src/dom/manipulator'
 
 describe('ScrollBar', () => {
   let fixtureEl
+  const doc = document.documentElement
   const parseInt = arg => Number.parseInt(arg, 10)
-  const getRightPadding = el => parseInt(window.getComputedStyle(el).paddingRight)
+  const getPaddingX = el => parseInt(window.getComputedStyle(el).paddingRight)
+  const getMarginX = el => parseInt(window.getComputedStyle(el).marginRight)
   const getOverFlow = el => el.style.overflow
   const getPaddingAttr = el => Manipulator.getDataAttribute(el, 'padding-right')
+  const getMarginAttr = el => Manipulator.getDataAttribute(el, 'margin-right')
   const getOverFlowAttr = el => Manipulator.getDataAttribute(el, 'overflow')
   const windowCalculations = () => {
     return {
@@ -61,8 +64,8 @@ describe('ScrollBar', () => {
       }
     })
 
-    it('should return false if body is overflowing', () => {
-      document.documentElement.style.overflowY = 'hidden'
+    it('should return false if body is not overflowing', () => {
+      doc.style.overflowY = 'hidden'
       document.body.style.overflowY = 'hidden'
       fixtureEl.innerHTML = [
         '<div style="height: 110vh; width: 100%"></div>'
@@ -75,7 +78,7 @@ describe('ScrollBar', () => {
 
   describe('getWidth', () => {
     it('should return an integer greater than zero, if body is overflowing', () => {
-      document.documentElement.style.overflowY = 'scroll'
+      doc.style.overflowY = 'scroll'
       document.body.style.overflowY = 'scroll'
       fixtureEl.innerHTML = [
         '<div style="height: 110vh; width: 100%"></div>'
@@ -110,56 +113,60 @@ describe('ScrollBar', () => {
         '<div class="fixed-top" id="fixed2" style="padding-right: 5px; width: 100vw"></div>',
         '</div>'
       ].join('')
-      document.documentElement.style.overflowY = 'scroll'
+      doc.style.overflowY = 'scroll'
 
       const fixedEl = fixtureEl.querySelector('#fixed1')
       const fixedEl2 = fixtureEl.querySelector('#fixed2')
-      const originalPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10)
-      const originalPadding2 = Number.parseInt(window.getComputedStyle(fixedEl2).paddingRight, 10)
+      const originalPadding = getPaddingX(fixedEl)
+      const originalPadding2 = getPaddingX(fixedEl2)
       const expectedPadding = originalPadding + Scrollbar.getWidth()
       const expectedPadding2 = originalPadding2 + Scrollbar.getWidth()
 
       Scrollbar.hide()
 
-      let currentPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10)
-      let currentPadding2 = Number.parseInt(window.getComputedStyle(fixedEl2).paddingRight, 10)
-      expect(fixedEl.getAttribute('data-bs-padding-right')).toEqual('0px', 'original fixed element padding should be stored in data-bs-padding-right')
-      expect(fixedEl2.getAttribute('data-bs-padding-right')).toEqual('5px', 'original fixed element padding should be stored in data-bs-padding-right')
+      let currentPadding = getPaddingX(fixedEl)
+      let currentPadding2 = getPaddingX(fixedEl2)
+      expect(getPaddingAttr(fixedEl)).toEqual(`${originalPadding}px`, 'original fixed element padding should be stored in data-bs-padding-right')
+      expect(getPaddingAttr(fixedEl2)).toEqual(`${originalPadding2}px`, 'original fixed element padding should be stored in data-bs-padding-right')
       expect(currentPadding).toEqual(expectedPadding, 'fixed element padding should be adjusted while opening')
       expect(currentPadding2).toEqual(expectedPadding2, 'fixed element padding should be adjusted while opening')
 
       Scrollbar.reset()
-      currentPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10)
-      currentPadding2 = Number.parseInt(window.getComputedStyle(fixedEl2).paddingRight, 10)
-      expect(fixedEl.getAttribute('data-bs-padding-right')).toEqual(null, 'data-bs-padding-right should be cleared after closing')
-      expect(fixedEl2.getAttribute('data-bs-padding-right')).toEqual(null, 'data-bs-padding-right should be cleared after closing')
+      currentPadding = getPaddingX(fixedEl)
+      currentPadding2 = getPaddingX(fixedEl2)
+      expect(getPaddingAttr(fixedEl)).toEqual(null, 'data-bs-padding-right should be cleared after closing')
+      expect(getPaddingAttr(fixedEl2)).toEqual(null, 'data-bs-padding-right should be cleared after closing')
       expect(currentPadding).toEqual(originalPadding, 'fixed element padding should be reset after closing')
       expect(currentPadding2).toEqual(originalPadding2, 'fixed element padding should be reset after closing')
       done()
     })
 
-    it('should adjust the inline margin of sticky elements', done => {
+    it('should adjust the inline margin and padding of sticky elements', done => {
       fixtureEl.innerHTML = [
         '<div style="height: 110vh">' +
-        '<div class="sticky-top" style="margin-right: 0px; width: 100vw; height: 10px"></div>',
+        '<div class="sticky-top" style="margin-right: 10px; padding-right: 20px; width: 100vw; height: 10px"></div>',
         '</div>'
       ].join('')
-      document.documentElement.style.overflowY = 'scroll'
+      doc.style.overflowY = 'scroll'
 
       const stickyTopEl = fixtureEl.querySelector('.sticky-top')
-      const originalMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10)
+      const originalMargin = getMarginX(stickyTopEl)
+      const originalPadding = getPaddingX(stickyTopEl)
+
       const expectedMargin = originalMargin - Scrollbar.getWidth()
+      const expectedPadding = originalPadding + Scrollbar.getWidth()
       Scrollbar.hide()
 
-      let currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10)
-      expect(stickyTopEl.getAttribute('data-bs-margin-right')).toEqual('0px', 'original sticky element margin should be stored in data-bs-margin-right')
-      expect(currentMargin).toEqual(expectedMargin, 'sticky element margin should be adjusted while opening')
+      expect(getMarginAttr(stickyTopEl)).toEqual(`${originalMargin}px`, 'original sticky element margin should be stored in data-bs-margin-right')
+      expect(getMarginX(stickyTopEl)).toEqual(expectedMargin, 'sticky element margin should be adjusted while opening')
+      expect(getPaddingAttr(stickyTopEl)).toEqual(`${originalPadding}px`, 'original sticky element margin should be stored in data-bs-margin-right')
+      expect(getPaddingX(stickyTopEl)).toEqual(expectedPadding, 'sticky element margin should be adjusted while opening')
 
       Scrollbar.reset()
-      currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10)
-
-      expect(stickyTopEl.getAttribute('data-bs-margin-right')).toEqual(null, 'data-bs-margin-right should be cleared after closing')
-      expect(currentMargin).toEqual(originalMargin, 'sticky element margin should be reset after closing')
+      expect(getMarginAttr(stickyTopEl)).toEqual(null, 'data-bs-margin-right should be cleared after closing')
+      expect(getMarginX(stickyTopEl)).toEqual(originalMargin, 'sticky element margin should be reset after closing')
+      expect(getPaddingAttr(stickyTopEl)).toEqual(null, 'data-bs-margin-right should be cleared after closing')
+      expect(getPaddingX(stickyTopEl)).toEqual(originalPadding, 'sticky element margin should be reset after closing')
       done()
     })
 
@@ -169,13 +176,13 @@ describe('ScrollBar', () => {
       ].join('')
 
       const stickyTopEl = fixtureEl.querySelector('.sticky-top')
-      const originalMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10)
-      const originalPadding = Number.parseInt(window.getComputedStyle(stickyTopEl).paddingRight, 10)
+      const originalMargin = getMarginX(stickyTopEl)
+      const originalPadding = getPaddingX(stickyTopEl)
 
       Scrollbar.hide()
 
-      const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10)
-      const currentPadding = Number.parseInt(window.getComputedStyle(stickyTopEl).paddingRight, 10)
+      const currentMargin = getMarginX(stickyTopEl)
+      const currentPadding = getPaddingX(stickyTopEl)
 
       expect(currentMargin).toEqual(originalMargin, 'sticky element\'s margin should not be adjusted while opening')
       expect(currentPadding).toEqual(originalPadding, 'sticky element\'s padding should not be adjusted while opening')
@@ -183,7 +190,49 @@ describe('ScrollBar', () => {
       Scrollbar.reset()
     })
 
+    it('should not put data-attribute if element doesn\'t have the proper style property, should just remove style property if element didn\'t had one', () => {
+      fixtureEl.innerHTML = [
+        '<div style="height: 110vh; width: 100%">' +
+        '<div class="sticky-top" id="sticky" style="width: 100vw"></div>',
+        '</div>'
+      ].join('')
+
+      document.body.style.overflowY = 'scroll'
+
+      const hasPaddingAttr = el => el.hasAttribute('data-bs-padding-right')
+      const hasMarginAttr = el => el.hasAttribute('data-bs-margin-right')
+      const stickyEl = fixtureEl.querySelector('#sticky')
+      const originalPadding = getPaddingX(stickyEl)
+      const originalMargin = getMarginX(stickyEl)
+      const scrollBarWidth = Scrollbar.getWidth()
+
+      Scrollbar.hide()
+
+      expect(getPaddingX(stickyEl)).toEqual(scrollBarWidth + originalPadding)
+      const expectedMargin = scrollBarWidth + originalMargin
+      expect(getMarginX(stickyEl)).toEqual(expectedMargin === 0 ? expectedMargin : -expectedMargin)
+      expect(hasMarginAttr(stickyEl)).toBeFalse() // We do not have to keep css margin
+      expect(hasPaddingAttr(stickyEl)).toBeFalse() // We do not have to keep css padding
+
+      Scrollbar.reset()
+
+      expect(getPaddingX(stickyEl)).toEqual(originalPadding)
+      expect(getPaddingX(stickyEl)).toEqual(originalPadding)
+    })
+
     describe('Body Handling', () => {
+      it('should ignore other inline styles when trying to restore body defaults ', () => {
+        document.body.style.color = 'red'
+
+        const scrollBarWidth = Scrollbar.getWidth()
+        Scrollbar.hide()
+
+        expect(getPaddingX(document.body)).toEqual(scrollBarWidth, 'body does not have inline padding set')
+        expect(document.body.style.color).toEqual('red', 'body still has other inline styles set')
+
+        Scrollbar.reset()
+      })
+
       it('should hide scrollbar and reset it to its initial value', () => {
         const styleSheetPadding = '7px'
         fixtureEl.innerHTML = [
@@ -198,7 +247,7 @@ describe('ScrollBar', () => {
         const inlineStylePadding = '10px'
         el.style.paddingRight = inlineStylePadding
 
-        const originalPadding = getRightPadding(el)
+        const originalPadding = getPaddingX(el)
         expect(originalPadding).toEqual(parseInt(inlineStylePadding)) // Respect only the inline style as it has prevails this of css
         const originalOverFlow = 'auto'
         el.style.overflow = originalOverFlow
@@ -206,7 +255,7 @@ describe('ScrollBar', () => {
 
         Scrollbar.hide()
 
-        const currentPadding = getRightPadding(el)
+        const currentPadding = getPaddingX(el)
 
         expect(currentPadding).toEqual(scrollBarWidth + originalPadding)
         expect(currentPadding).toEqual(scrollBarWidth + parseInt(inlineStylePadding))
@@ -216,7 +265,7 @@ describe('ScrollBar', () => {
 
         Scrollbar.reset()
 
-        const currentPadding1 = getRightPadding(el)
+        const currentPadding1 = getPaddingX(el)
         expect(currentPadding1).toEqual(originalPadding)
         expect(getPaddingAttr(el)).toEqual(null)
         expect(getOverFlow(el)).toEqual(originalOverFlow)
@@ -233,14 +282,14 @@ describe('ScrollBar', () => {
           '</style>'
         ].join('')
         const el = document.body
-        const originalPadding = getRightPadding(el)
+        const originalPadding = getPaddingX(el)
         const originalOverFlow = 'scroll'
         el.style.overflow = originalOverFlow
         const scrollBarWidth = Scrollbar.getWidth()
 
         Scrollbar.hide()
 
-        const currentPadding = getRightPadding(el)
+        const currentPadding = getPaddingX(el)
 
         expect(currentPadding).toEqual(scrollBarWidth + originalPadding)
         expect(currentPadding).toEqual(scrollBarWidth + parseInt(styleSheetPadding))
@@ -250,12 +299,46 @@ describe('ScrollBar', () => {
 
         Scrollbar.reset()
 
-        const currentPadding1 = getRightPadding(el)
+        const currentPadding1 = getPaddingX(el)
         expect(currentPadding1).toEqual(originalPadding)
         expect(getPaddingAttr(el)).toEqual(null)
         expect(getOverFlow(el)).toEqual(originalOverFlow)
         expect(getOverFlowAttr(el)).toEqual(null)
       })
+
+      it('should not adjust the inline body padding when it does not overflow', () => {
+        const originalPadding = getPaddingX(document.body)
+
+        // Hide scrollbars to prevent the body overflowing
+        doc.style.overflowY = 'hidden'
+        doc.style.paddingRight = '0px'
+
+        Scrollbar.hide()
+        const currentPadding = getPaddingX(document.body)
+
+        expect(currentPadding).toEqual(originalPadding, 'body padding should not be adjusted')
+        Scrollbar.reset()
+      })
+
+      it('should not adjust the inline body padding when it does not overflow, even on a scaled display', () => {
+        const originalPadding = getPaddingX(document.body)
+        // Remove body margins as would be done by Bootstrap css
+        document.body.style.margin = '0'
+
+        // Hide scrollbars to prevent the body overflowing
+        doc.style.overflowY = 'hidden'
+
+        // Simulate a discrepancy between exact, i.e. floating point body width, and rounded body width
+        // as it can occur when zooming or scaling the display to something else than 100%
+        doc.style.paddingRight = '.48px'
+        Scrollbar.hide()
+
+        const currentPadding = getPaddingX(document.body)
+
+        expect(currentPadding).toEqual(originalPadding, 'body padding should not be adjusted')
+
+        Scrollbar.reset()
+      })
     })
   })
 })