Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scrollbar: respect the initial body overflow value #33706

Merged
merged 2 commits into from Apr 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 14 additions & 4 deletions js/src/util/scrollbar.js
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
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
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')
GeoSot marked this conversation as resolved.
Show resolved Hide resolved
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
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
@@ -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
GeoSot marked this conversation as resolved.
Show resolved Hide resolved
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)
})
})
})
})