Skip to content

Commit

Permalink
Scrollbar: respect the initial body overflow value (#33706)
Browse files Browse the repository at this point in the history 
* add method to handle overflow on body element & tests
* replace duplicated code on modal/offcanvas tests
  • Loading branch information
@GeoSot
GeoSot committed Apr 25, 2021
1 parent e2294ff commit d381820
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 26 deletions.
18 changes: 14 additions & 4 deletions js/src/util/scrollbar.js
View file
Expand Up @@ -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) => {
Expand All @@ -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) => {
Expand Down
9 changes: 9 additions & 0 deletions js/tests/helpers/fixture.js
View file
Expand Up @@ -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)
})
}
11 changes: 3 additions & 8 deletions js/tests/unit/modal.spec.js
View file
Expand Up @@ -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
Expand All @@ -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 => {
Expand All @@ -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', () => {
Expand Down
10 changes: 3 additions & 7 deletions js/tests/unit/offcanvas.spec.js
View file
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down
91 changes: 84 additions & 7 deletions js/tests/unit/util/scrollbar.spec.js
View file
@@ -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,
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 = [
'<style>',
' body {',
` padding-right: ${styleSheetPadding} }`,
' }',
'</style>'
].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 = [
'<style>',
' body {',
` padding-right: ${styleSheetPadding} }`,
' }',
'</style>'
].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)
})
})
})
})

0 comments on commit d381820

Please sign in to comment.