From d381820d1678044257fe2cb7b2f178d1f001ed30 Mon Sep 17 00:00:00 2001 From: GeoSot Date: Sun, 25 Apr 2021 06:50:16 +0300 Subject: [PATCH] Scrollbar: respect the initial body overflow value (#33706) * add method to handle overflow on body element & tests * replace duplicated code on modal/offcanvas tests --- js/src/util/scrollbar.js | 18 ++++-- js/tests/helpers/fixture.js | 9 +++ js/tests/unit/modal.spec.js | 11 +--- js/tests/unit/offcanvas.spec.js | 10 +-- js/tests/unit/util/scrollbar.spec.js | 91 +++++++++++++++++++++++++--- 5 files changed, 113 insertions(+), 26 deletions(-) diff --git a/js/src/util/scrollbar.js b/js/src/util/scrollbar.js index 31b6143756..352e3e11de 100644 --- a/js/src/util/scrollbar.js +++ b/js/src/util/scrollbar.js @@ -18,11 +18,21 @@ const getWidth = () => { } const hide = (width = getWidth()) => { - document.body.style.overflow = 'hidden' + _disableOverFlow() + // give padding to element to balances the hidden scrollbar width + _setElementAttributes('body', 'paddingRight', calculatedValue => calculatedValue + width) // trick: We adjust positive paddingRight and negative marginRight to sticky-top elements, to keep shown fullwidth _setElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight', calculatedValue => calculatedValue + width) _setElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight', calculatedValue => calculatedValue - width) - _setElementAttributes('body', 'paddingRight', calculatedValue => calculatedValue + width) +} + +const _disableOverFlow = () => { + const actualValue = document.body.style.overflow + if (actualValue) { + Manipulator.setDataAttribute(document.body, 'overflow', actualValue) + } + + document.body.style.overflow = 'hidden' } const _setElementAttributes = (selector, styleProp, callback) => { @@ -41,10 +51,10 @@ const _setElementAttributes = (selector, styleProp, callback) => { } const reset = () => { - document.body.style.overflow = 'auto' + _resetElementAttributes('body', 'overflow') + _resetElementAttributes('body', 'paddingRight') _resetElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight') _resetElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight') - _resetElementAttributes('body', 'paddingRight') } const _resetElementAttributes = (selector, styleProp) => { diff --git a/js/tests/helpers/fixture.js b/js/tests/helpers/fixture.js index 0bfc26f468..3d6f395e8f 100644 --- a/js/tests/helpers/fixture.js +++ b/js/tests/helpers/fixture.js @@ -39,3 +39,12 @@ export const jQueryMock = { }) } } + +export const clearBodyAndDocument = () => { + const attributes = ['data-bs-padding-right', 'style'] + + attributes.forEach(attr => { + document.documentElement.removeAttribute(attr) + document.body.removeAttribute(attr) + }) +} diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index 624f44dc5d..a09711b349 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -3,7 +3,7 @@ import EventHandler from '../../src/dom/event-handler' import { getWidth as getScrollBarWidth } from '../../src/util/scrollbar' /** Test helpers */ -import { clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture' +import { clearBodyAndDocument, clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture' describe('Modal', () => { let fixtureEl @@ -14,11 +14,8 @@ describe('Modal', () => { afterEach(() => { clearFixture() - + clearBodyAndDocument() document.body.classList.remove('modal-open') - document.documentElement.removeAttribute('style') - document.body.removeAttribute('style') - document.body.removeAttribute('data-bs-padding-right') document.querySelectorAll('.modal-backdrop') .forEach(backdrop => { @@ -27,9 +24,7 @@ describe('Modal', () => { }) beforeEach(() => { - document.documentElement.removeAttribute('style') - document.body.removeAttribute('style') - document.body.removeAttribute('data-bs-padding-right') + clearBodyAndDocument() }) describe('VERSION', () => { diff --git a/js/tests/unit/offcanvas.spec.js b/js/tests/unit/offcanvas.spec.js index 2419e5723c..30edc2913b 100644 --- a/js/tests/unit/offcanvas.spec.js +++ b/js/tests/unit/offcanvas.spec.js @@ -2,7 +2,7 @@ import Offcanvas from '../../src/offcanvas' import EventHandler from '../../src/dom/event-handler' /** Test helpers */ -import { clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture' +import { clearBodyAndDocument, clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture' import { isVisible } from '../../src/util' describe('Offcanvas', () => { @@ -15,15 +15,11 @@ describe('Offcanvas', () => { afterEach(() => { clearFixture() document.body.classList.remove('offcanvas-open') - document.documentElement.removeAttribute('style') - document.body.removeAttribute('style') - document.body.removeAttribute('data-bs-padding-right') + clearBodyAndDocument() }) beforeEach(() => { - document.documentElement.removeAttribute('style') - document.body.removeAttribute('style') - document.body.removeAttribute('data-bs-padding-right') + clearBodyAndDocument() }) describe('VERSION', () => { diff --git a/js/tests/unit/util/scrollbar.spec.js b/js/tests/unit/util/scrollbar.spec.js index aab3798eea..e09a37ce70 100644 --- a/js/tests/unit/util/scrollbar.spec.js +++ b/js/tests/unit/util/scrollbar.spec.js @@ -1,8 +1,14 @@ import * as Scrollbar from '../../../src/util/scrollbar' -import { clearFixture, getFixture } from '../../helpers/fixture' +import { clearBodyAndDocument, clearFixture, getFixture } from '../../helpers/fixture' +import Manipulator from '../../../src/dom/manipulator' describe('ScrollBar', () => { let fixtureEl + const parseInt = arg => Number.parseInt(arg, 10) + const getRightPadding = el => parseInt(window.getComputedStyle(el).paddingRight) + const getOverFlow = el => el.style.overflow + const getPaddingAttr = el => Manipulator.getDataAttribute(el, 'padding-right') + const getOverFlowAttr = el => Manipulator.getDataAttribute(el, 'overflow') const windowCalculations = () => { return { htmlClient: document.documentElement.clientWidth, @@ -32,15 +38,11 @@ describe('ScrollBar', () => { afterEach(() => { clearFixture() - document.documentElement.removeAttribute('style') - document.body.removeAttribute('style') - document.body.removeAttribute('data-bs-padding-right') + clearBodyAndDocument() }) beforeEach(() => { - document.documentElement.removeAttribute('style') - document.body.removeAttribute('style') - document.body.removeAttribute('data-bs-padding-right') + clearBodyAndDocument() }) describe('isBodyOverflowing', () => { @@ -180,5 +182,80 @@ describe('ScrollBar', () => { Scrollbar.reset() }) + + describe('Body Handling', () => { + it('should hide scrollbar and reset it to its initial value', () => { + const styleSheetPadding = '7px' + fixtureEl.innerHTML = [ + '' + ].join('') + + const el = document.body + const inlineStylePadding = '10px' + el.style.paddingRight = inlineStylePadding + + const originalPadding = getRightPadding(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 + const scrollBarWidth = Scrollbar.getWidth() + + Scrollbar.hide() + + const currentPadding = getRightPadding(el) + + expect(currentPadding).toEqual(scrollBarWidth + originalPadding) + expect(currentPadding).toEqual(scrollBarWidth + parseInt(inlineStylePadding)) + expect(getPaddingAttr(el)).toEqual(inlineStylePadding) + expect(getOverFlow(el)).toEqual('hidden') + expect(getOverFlowAttr(el)).toEqual(originalOverFlow) + + Scrollbar.reset() + + const currentPadding1 = getRightPadding(el) + expect(currentPadding1).toEqual(originalPadding) + expect(getPaddingAttr(el)).toEqual(null) + expect(getOverFlow(el)).toEqual(originalOverFlow) + expect(getOverFlowAttr(el)).toEqual(null) + }) + + it('should hide scrollbar and reset it to its initial value - respecting css rules', () => { + const styleSheetPadding = '7px' + fixtureEl.innerHTML = [ + '' + ].join('') + const el = document.body + const originalPadding = getRightPadding(el) + const originalOverFlow = 'scroll' + el.style.overflow = originalOverFlow + const scrollBarWidth = Scrollbar.getWidth() + + Scrollbar.hide() + + const currentPadding = getRightPadding(el) + + expect(currentPadding).toEqual(scrollBarWidth + originalPadding) + expect(currentPadding).toEqual(scrollBarWidth + parseInt(styleSheetPadding)) + expect(getPaddingAttr(el)).toBeNull() // We do not have to keep css padding + expect(getOverFlow(el)).toEqual('hidden') + expect(getOverFlowAttr(el)).toEqual(originalOverFlow) + + Scrollbar.reset() + + const currentPadding1 = getRightPadding(el) + expect(currentPadding1).toEqual(originalPadding) + expect(getPaddingAttr(el)).toEqual(null) + expect(getOverFlow(el)).toEqual(originalOverFlow) + expect(getOverFlowAttr(el)).toEqual(null) + }) + }) }) })