Skip to content

Commit d381820

Browse files
GeoSotGeoSot
authored
Scrollbar: respect the initial body overflow value (#33706)
* add method to handle overflow on body element & tests * replace duplicated code on modal/offcanvas tests
1 parent e2294ff commit d381820

File tree

5 files changed

+113
-26
lines changed

5 files changed

+113
-26
lines changed

‎js/src/util/scrollbar.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,21 @@ const getWidth = () => {
1818
}
1919

2020
const hide = (width = getWidth()) => {
21-
document.body.style.overflow = 'hidden'
21+
_disableOverFlow()
22+
// give padding to element to balances the hidden scrollbar width
23+
_setElementAttributes('body', 'paddingRight', calculatedValue => calculatedValue + width)
2224
// trick: We adjust positive paddingRight and negative marginRight to sticky-top elements, to keep shown fullwidth
2325
_setElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight', calculatedValue => calculatedValue + width)
2426
_setElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight', calculatedValue => calculatedValue - width)
25-
_setElementAttributes('body', 'paddingRight', calculatedValue => calculatedValue + width)
27+
}
28+
29+
const _disableOverFlow = () => {
30+
const actualValue = document.body.style.overflow
31+
if (actualValue) {
32+
Manipulator.setDataAttribute(document.body, 'overflow', actualValue)
33+
}
34+
35+
document.body.style.overflow = 'hidden'
2636
}
2737

2838
const _setElementAttributes = (selector, styleProp, callback) => {
@@ -41,10 +51,10 @@ const _setElementAttributes = (selector, styleProp, callback) => {
4151
}
4252

4353
const reset = () => {
44-
document.body.style.overflow = 'auto'
54+
_resetElementAttributes('body', 'overflow')
55+
_resetElementAttributes('body', 'paddingRight')
4556
_resetElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight')
4657
_resetElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight')
47-
_resetElementAttributes('body', 'paddingRight')
4858
}
4959

5060
const _resetElementAttributes = (selector, styleProp) => {

‎js/tests/helpers/fixture.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,12 @@ export const jQueryMock = {
3939
})
4040
}
4141
}
42+
43+
export const clearBodyAndDocument = () => {
44+
const attributes = ['data-bs-padding-right', 'style']
45+
46+
attributes.forEach(attr => {
47+
document.documentElement.removeAttribute(attr)
48+
document.body.removeAttribute(attr)
49+
})
50+
}

‎js/tests/unit/modal.spec.js

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import EventHandler from '../../src/dom/event-handler'
33
import { getWidth as getScrollBarWidth } from '../../src/util/scrollbar'
44

55
/** Test helpers */
6-
import { clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture'
6+
import { clearBodyAndDocument, clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture'
77

88
describe('Modal', () => {
99
let fixtureEl
@@ -14,11 +14,8 @@ describe('Modal', () => {
1414

1515
afterEach(() => {
1616
clearFixture()
17-
17+
clearBodyAndDocument()
1818
document.body.classList.remove('modal-open')
19-
document.documentElement.removeAttribute('style')
20-
document.body.removeAttribute('style')
21-
document.body.removeAttribute('data-bs-padding-right')
2219

2320
document.querySelectorAll('.modal-backdrop')
2421
.forEach(backdrop => {
@@ -27,9 +24,7 @@ describe('Modal', () => {
2724
})
2825

2926
beforeEach(() => {
30-
document.documentElement.removeAttribute('style')
31-
document.body.removeAttribute('style')
32-
document.body.removeAttribute('data-bs-padding-right')
27+
clearBodyAndDocument()
3328
})
3429

3530
describe('VERSION', () => {

‎js/tests/unit/offcanvas.spec.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import Offcanvas from '../../src/offcanvas'
22
import EventHandler from '../../src/dom/event-handler'
33

44
/** Test helpers */
5-
import { clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture'
5+
import { clearBodyAndDocument, clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture'
66
import { isVisible } from '../../src/util'
77

88
describe('Offcanvas', () => {
@@ -15,15 +15,11 @@ describe('Offcanvas', () => {
1515
afterEach(() => {
1616
clearFixture()
1717
document.body.classList.remove('offcanvas-open')
18-
document.documentElement.removeAttribute('style')
19-
document.body.removeAttribute('style')
20-
document.body.removeAttribute('data-bs-padding-right')
18+
clearBodyAndDocument()
2119
})
2220

2321
beforeEach(() => {
24-
document.documentElement.removeAttribute('style')
25-
document.body.removeAttribute('style')
26-
document.body.removeAttribute('data-bs-padding-right')
22+
clearBodyAndDocument()
2723
})
2824

2925
describe('VERSION', () => {

‎js/tests/unit/util/scrollbar.spec.js

Lines changed: 84 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
import * as Scrollbar from '../../../src/util/scrollbar'
2-
import { clearFixture, getFixture } from '../../helpers/fixture'
2+
import { clearBodyAndDocument, clearFixture, getFixture } from '../../helpers/fixture'
3+
import Manipulator from '../../../src/dom/manipulator'
34

45
describe('ScrollBar', () => {
56
let fixtureEl
7+
const parseInt = arg => Number.parseInt(arg, 10)
8+
const getRightPadding = el => parseInt(window.getComputedStyle(el).paddingRight)
9+
const getOverFlow = el => el.style.overflow
10+
const getPaddingAttr = el => Manipulator.getDataAttribute(el, 'padding-right')
11+
const getOverFlowAttr = el => Manipulator.getDataAttribute(el, 'overflow')
612
const windowCalculations = () => {
713
return {
814
htmlClient: document.documentElement.clientWidth,
@@ -32,15 +38,11 @@ describe('ScrollBar', () => {
3238

3339
afterEach(() => {
3440
clearFixture()
35-
document.documentElement.removeAttribute('style')
36-
document.body.removeAttribute('style')
37-
document.body.removeAttribute('data-bs-padding-right')
41+
clearBodyAndDocument()
3842
})
3943

4044
beforeEach(() => {
41-
document.documentElement.removeAttribute('style')
42-
document.body.removeAttribute('style')
43-
document.body.removeAttribute('data-bs-padding-right')
45+
clearBodyAndDocument()
4446
})
4547

4648
describe('isBodyOverflowing', () => {
@@ -180,5 +182,80 @@ describe('ScrollBar', () => {
180182

181183
Scrollbar.reset()
182184
})
185+
186+
describe('Body Handling', () => {
187+
it('should hide scrollbar and reset it to its initial value', () => {
188+
const styleSheetPadding = '7px'
189+
fixtureEl.innerHTML = [
190+
'<style>',
191+
' body {',
192+
` padding-right: ${styleSheetPadding} }`,
193+
' }',
194+
'</style>'
195+
].join('')
196+
197+
const el = document.body
198+
const inlineStylePadding = '10px'
199+
el.style.paddingRight = inlineStylePadding
200+
201+
const originalPadding = getRightPadding(el)
202+
expect(originalPadding).toEqual(parseInt(inlineStylePadding)) // Respect only the inline style as it has prevails this of css
203+
const originalOverFlow = 'auto'
204+
el.style.overflow = originalOverFlow
205+
const scrollBarWidth = Scrollbar.getWidth()
206+
207+
Scrollbar.hide()
208+
209+
const currentPadding = getRightPadding(el)
210+
211+
expect(currentPadding).toEqual(scrollBarWidth + originalPadding)
212+
expect(currentPadding).toEqual(scrollBarWidth + parseInt(inlineStylePadding))
213+
expect(getPaddingAttr(el)).toEqual(inlineStylePadding)
214+
expect(getOverFlow(el)).toEqual('hidden')
215+
expect(getOverFlowAttr(el)).toEqual(originalOverFlow)
216+
217+
Scrollbar.reset()
218+
219+
const currentPadding1 = getRightPadding(el)
220+
expect(currentPadding1).toEqual(originalPadding)
221+
expect(getPaddingAttr(el)).toEqual(null)
222+
expect(getOverFlow(el)).toEqual(originalOverFlow)
223+
expect(getOverFlowAttr(el)).toEqual(null)
224+
})
225+
226+
it('should hide scrollbar and reset it to its initial value - respecting css rules', () => {
227+
const styleSheetPadding = '7px'
228+
fixtureEl.innerHTML = [
229+
'<style>',
230+
' body {',
231+
` padding-right: ${styleSheetPadding} }`,
232+
' }',
233+
'</style>'
234+
].join('')
235+
const el = document.body
236+
const originalPadding = getRightPadding(el)
237+
const originalOverFlow = 'scroll'
238+
el.style.overflow = originalOverFlow
239+
const scrollBarWidth = Scrollbar.getWidth()
240+
241+
Scrollbar.hide()
242+
243+
const currentPadding = getRightPadding(el)
244+
245+
expect(currentPadding).toEqual(scrollBarWidth + originalPadding)
246+
expect(currentPadding).toEqual(scrollBarWidth + parseInt(styleSheetPadding))
247+
expect(getPaddingAttr(el)).toBeNull() // We do not have to keep css padding
248+
expect(getOverFlow(el)).toEqual('hidden')
249+
expect(getOverFlowAttr(el)).toEqual(originalOverFlow)
250+
251+
Scrollbar.reset()
252+
253+
const currentPadding1 = getRightPadding(el)
254+
expect(currentPadding1).toEqual(originalPadding)
255+
expect(getPaddingAttr(el)).toEqual(null)
256+
expect(getOverFlow(el)).toEqual(originalOverFlow)
257+
expect(getOverFlowAttr(el)).toEqual(null)
258+
})
259+
})
183260
})
184261
})

0 commit comments

Comments
 (0)