ScrollBar.js. Minor refactoring and add test (#35492)

2 files changed, 38 insertions, 13 deletions

diff --git a/js/src/util/scrollbar.js b/js/src/util/scrollbar.js
index 55b7244ab3..187a6694dc 100644
--- a/js/src/util/scrollbar.js
+++ b/js/src/util/scrollbar.js
@@ -15,6 +15,8 @@ import { isElement } from './index'
 
 const SELECTOR_FIXED_CONTENT = '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top'
 const SELECTOR_STICKY_CONTENT = '.sticky-top'
+const PROPERTY_PADDING = 'paddingRight'
+const PROPERTY_MARGIN = 'marginRight'
 
 /**
  * Class definition
@@ -36,17 +38,17 @@ class ScrollBarHelper {
     const width = this.getWidth()
     this._disableOverFlow()
     // give padding to element to balance the hidden scrollbar width
-    this._setElementAttributes(this._element, 'paddingRight', calculatedValue => calculatedValue + width)
+    this._setElementAttributes(this._element, PROPERTY_PADDING, calculatedValue => calculatedValue + width)
     // trick: We adjust positive paddingRight and negative marginRight to sticky-top elements to keep showing fullwidth
-    this._setElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight', calculatedValue => calculatedValue + width)
-    this._setElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight', calculatedValue => calculatedValue - width)
+    this._setElementAttributes(SELECTOR_FIXED_CONTENT, PROPERTY_PADDING, calculatedValue => calculatedValue + width)
+    this._setElementAttributes(SELECTOR_STICKY_CONTENT, PROPERTY_MARGIN, calculatedValue => calculatedValue - width)
   }
 
   reset() {
     this._resetElementAttributes(this._element, 'overflow')
-    this._resetElementAttributes(this._element, 'paddingRight')
-    this._resetElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight')
-    this._resetElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight')
+    this._resetElementAttributes(this._element, PROPERTY_PADDING)
+    this._resetElementAttributes(SELECTOR_FIXED_CONTENT, PROPERTY_PADDING)
+    this._resetElementAttributes(SELECTOR_STICKY_CONTENT, PROPERTY_MARGIN)
   }
 
   isOverflowing() {
@@ -86,10 +88,11 @@ class ScrollBarHelper {
       const value = Manipulator.getDataAttribute(element, styleProp)
       if (typeof value === 'undefined') {
         element.style.removeProperty(styleProp)
-      } else {
-        Manipulator.removeDataAttribute(element, styleProp)
-        element.style[styleProp] = value
+        return
       }
+
+      Manipulator.removeDataAttribute(element, styleProp)
+      element.style[styleProp] = value
     }
 
     this._applyManipulationCallback(selector, manipulationCallBack)
@@ -98,10 +101,11 @@ class ScrollBarHelper {
   _applyManipulationCallback(selector, callBack) {
     if (isElement(selector)) {
       callBack(selector)
-    } else {
-      for (const sel of SelectorEngine.find(selector, this._element)) {
-        callBack(sel)
-      }
+      return
+    }
+
+    for (const sel of SelectorEngine.find(selector, this._element)) {
+      callBack(sel)
     }
   }
 }
diff --git a/js/tests/unit/util/scrollbar.spec.js b/js/tests/unit/util/scrollbar.spec.js
index 15f09c0b22..fc3a6f4e87 100644
--- a/js/tests/unit/util/scrollbar.spec.js
+++ b/js/tests/unit/util/scrollbar.spec.js
@@ -137,6 +137,27 @@ describe('ScrollBar', () => {
       done()
     })
 
+    it('should remove padding & margin if not existed before adjustment', done => {
+      fixtureEl.innerHTML = [
+        '<div style="height: 110vh; width: 100%">',
+        '  <div class="fixed" id="fixed" style="width: 100vw;"></div>',
+        '  <div class="sticky-top" id="sticky" style=" width: 100vw;"></div>',
+        '</div>'
+      ].join('')
+      doc.style.overflowY = 'scroll'
+
+      const fixedEl = fixtureEl.querySelector('#fixed')
+      const stickyEl = fixtureEl.querySelector('#sticky')
+      const scrollBar = new ScrollBarHelper()
+
+      scrollBar.hide()
+      scrollBar.reset()
+
+      expect(fixedEl.getAttribute('style').includes('padding-right')).toBeFalse()
+      expect(stickyEl.getAttribute('style').includes('margin-right')).toBeFalse()
+      done()
+    })
+
     it('should adjust the inline margin and padding of sticky elements', done => {
       fixtureEl.innerHTML = [
         '<div style="height: 110vh">',