refactor scrollbar method, plus tests

GeoSot · Mar 2, 2021 · f7f35fb · f7f35fb
1 parent 3eb6b34
commit f7f35fb
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 24 deletions.
diff --git a/js/src/util/scrollbar.js b/js/src/util/scrollbar.js
@@ -13,7 +13,9 @@ const SELECTOR_STICKY_CONTENT = '.sticky-top'
 
 const getWidth = () => {
   // https://muffinman.io/blog/get-scrollbar-width-in-javascript/
-  return Math.abs(window.innerWidth - document.documentElement.clientWidth)
+  const documentWidth = document.documentElement.clientWidth
+  // const documentWidth = document.body.getBoundingClientRect().width
+  return Math.abs(window.innerWidth - documentWidth)
 }
 
 const hide = (width = getWidth()) => {
@@ -53,9 +55,7 @@ const _resetElementAttributes = (selector, styleProp) => {
 }
 
 const isBodyOverflowing = () => {
-  // maybe getWidth > 0
-  const rect = document.body.getBoundingClientRect()
-  return Math.round(rect.left + rect.right) < window.innerWidth
+  return getWidth() > 0
 }
 
 export {

diff --git a/js/tests/unit/util/scrollbar.spec.js b/js/tests/unit/util/scrollbar.spec.js
@@ -10,29 +10,26 @@ describe('ScrollBar', () => {
 
   afterEach(() => {
     clearFixture()
-    document.body.style.overflowY = 'auto'
+    document.body.style.removeProperty('overflow')
+    document.body.removeAttribute('style')
   })
 
   describe('isBodyOverflowing', () => {
     it('should return true if body is overflowing', () => {
+      document.body.style.overflowY = 'scroll'
       fixtureEl.innerHTML = [
-        '<div style="height: 110vh">' +
-        '<div style="width: 100%; height: 80px"></div>' +
-        '</div>'
+        '<div style="height: 110vh; width: 100%"></div>'
       ].join('')
-      document.body.style.overflowY = 'scroll'
       const result = Scrollbar.isBodyOverflowing()
 
       expect(result).toEqual(true)
     })
 
     it('should return false if body is overflowing', () => {
+      document.body.style.overflowY = 'hidden'
       fixtureEl.innerHTML = [
-        '<div style="height: 90vh">' +
-        '<div style="width: 100%; height: 80px"></div>' +
-        '</div>'
+        '<div style="height: 110vh; width: 100%"></div>'
       ].join('')
-      document.body.style.overflowY = 'hidden'
 
       const result = Scrollbar.isBodyOverflowing()
 
@@ -41,25 +38,21 @@ describe('ScrollBar', () => {
   })
 
   describe('getWidth', () => {
-    it('should return an integer if body is overflowing', () => {
+    it('should return an integer greater than zero, if body is overflowing', () => {
+      document.body.style.overflowY = 'scroll'
       fixtureEl.innerHTML = [
-        '<div style="height: 110vh">' +
-        '<div style="width: 100%; height: 80px"></div>' +
-        '</div>'
+        '<div style="height: 110vh; width: 100%"></div>'
       ].join('')
-      document.body.style.overflowY = 'scroll'
       const result = Scrollbar.getWidth()
 
       expect(result).toBeGreaterThan(1)
     })
 
-    it('should return 0 if body is overflowing', () => {
+    it('should return 0 if body is hot overflowing', () => {
+      document.body.style.overflowY = 'hidden'
       fixtureEl.innerHTML = [
-        '<div style="height: 90vh">' +
-        '<div style="width: 100%; height: 80px"></div>' +
-        '</div>'
+        '<div style="height: 110vh; width: 100%"></div>'
       ].join('')
-      document.body.style.overflowY = 'hidden'
 
       const result = Scrollbar.getWidth()
 
@@ -70,7 +63,7 @@ describe('ScrollBar', () => {
   describe('hide - reset', () => {
     it('should adjust the inline padding of fixed elements', done => {
       fixtureEl.innerHTML = [
-        '<div style="height: 110vh">' +
+        '<div style="height: 110vh; width: 100%">' +
         '<div class="fixed-top" style="padding-right: 0px"></div>',
         '</div>'
       ].join('')