diff --git a/js/src/util/scrollbar.js b/js/src/util/scrollbar.js index 98c076f25f47..79a3b12c78a4 100644 --- a/js/src/util/scrollbar.js +++ b/js/src/util/scrollbar.js @@ -44,8 +44,11 @@ const _setElementAttributes = (selector, styleProp, callback) => { } const actualValue = element.style[styleProp] + if (actualValue) { + Manipulator.setDataAttribute(element, styleProp, actualValue) + } + const calculatedValue = window.getComputedStyle(element)[styleProp] - Manipulator.setDataAttribute(element, styleProp, actualValue) element.style[styleProp] = `${callback(Number.parseFloat(calculatedValue))}px` }) } diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index 799152e64c05..92bd1423ec27 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -1,6 +1,5 @@ import Modal from '../../src/modal' import EventHandler from '../../src/dom/event-handler' -import { getWidth as getScrollBarWidth } from '../../src/util/scrollbar' /** Test helpers */ import { clearBodyAndDocument, clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture' @@ -62,170 +61,22 @@ describe('Modal', () => { it('should toggle a modal', done => { fixtureEl.innerHTML = '' - document.documentElement.style.overflowY = 'scroll' + const initialOverFlow = document.body.style.overflow const modalEl = fixtureEl.querySelector('.modal') const modal = new Modal(modalEl) - const originalPadding = '0px' + const originalPadding = '10px' document.body.style.paddingRight = originalPadding modalEl.addEventListener('shown.bs.modal', () => { expect(document.body.getAttribute('data-bs-padding-right')).toEqual(originalPadding, 'original body padding should be stored in data-bs-padding-right') + expect(document.body.style.overflow).toEqual('hidden') modal.toggle() }) modalEl.addEventListener('hidden.bs.modal', () => { expect(document.body.getAttribute('data-bs-padding-right')).toBeNull() - expect().nothing() - document.documentElement.style.overflowY = 'auto' - done() - }) - - modal.toggle() - }) - - it('should adjust the inline padding of fixed elements when opening and restore when closing', done => { - fixtureEl.innerHTML = [ - '
', - '' - ].join('') - - document.documentElement.style.overflowY = 'scroll' - const fixedEl = fixtureEl.querySelector('.fixed-top') - const originalPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10) - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - const scrollBarWidth = getScrollBarWidth() - - modalEl.addEventListener('shown.bs.modal', () => { - const expectedPadding = originalPadding + scrollBarWidth - const currentPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10) - - expect(fixedEl.getAttribute('data-bs-padding-right')).toEqual(`${originalPadding}px`, 'original fixed element padding should be stored in data-bs-padding-right') - expect(currentPadding).toEqual(expectedPadding, 'fixed element padding should be adjusted while opening') - modal.toggle() - }) - - modalEl.addEventListener('hidden.bs.modal', () => { - const currentPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10) - - expect(fixedEl.hasAttribute('data-bs-padding-right')).toEqual(false, 'data-bs-padding-right should be cleared after closing') - expect(currentPadding).toEqual(originalPadding, 'fixed element padding should be reset after closing') - document.documentElement.style.overflowY = 'auto' - done() - }) - - modal.toggle() - }) - - it('should adjust the inline margin of sticky elements when opening and restore when closing', done => { - fixtureEl.innerHTML = [ - '
', - '' - ].join('') - - document.documentElement.style.overflowY = 'scroll' - - const stickyTopEl = fixtureEl.querySelector('.sticky-top') - const originalMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - const scrollBarWidth = getScrollBarWidth() - - modalEl.addEventListener('shown.bs.modal', () => { - const expectedMargin = originalMargin - scrollBarWidth - const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - - expect(stickyTopEl.getAttribute('data-bs-margin-right')).toEqual(`${originalMargin}px`, 'original sticky element margin should be stored in data-bs-margin-right') - expect(currentMargin).toEqual(expectedMargin, 'sticky element margin should be adjusted while opening') - modal.toggle() - }) - - modalEl.addEventListener('hidden.bs.modal', () => { - const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - - expect(stickyTopEl.hasAttribute('data-bs-margin-right')).toEqual(false, 'data-bs-margin-right should be cleared after closing') - expect(currentMargin).toEqual(originalMargin, 'sticky element margin should be reset after closing') - - document.documentElement.style.overflowY = 'auto' - done() - }) - - modal.toggle() - }) - - it('should not adjust the inline margin and padding of sticky and fixed elements when element do not have full width', done => { - fixtureEl.innerHTML = [ - '
', - '' - ].join('') - - const stickyTopEl = fixtureEl.querySelector('.sticky-top') - const originalMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - const originalPadding = Number.parseInt(window.getComputedStyle(stickyTopEl).paddingRight, 10) - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - - modalEl.addEventListener('shown.bs.modal', () => { - const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - const currentPadding = Number.parseInt(window.getComputedStyle(stickyTopEl).paddingRight, 10) - - expect(currentMargin).toEqual(originalMargin, 'sticky element\'s margin should not be adjusted while opening') - expect(currentPadding).toEqual(originalPadding, 'sticky element\'s padding should not be adjusted while opening') - done() - }) - - modal.show() - }) - - it('should ignore values set via CSS when trying to restore body padding after closing', done => { - fixtureEl.innerHTML = '' - const styleTest = document.createElement('style') - - styleTest.type = 'text/css' - styleTest.appendChild(document.createTextNode('body { padding-right: 7px; }')) - document.head.appendChild(styleTest) - - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - - modalEl.addEventListener('shown.bs.modal', () => { - modal.toggle() - }) - - modalEl.addEventListener('hidden.bs.modal', () => { - expect(window.getComputedStyle(document.body).paddingLeft).toEqual('0px', 'body does not have inline padding set') - document.head.removeChild(styleTest) - done() - }) - - modal.toggle() - }) - - it('should ignore other inline styles when trying to restore body padding after closing', done => { - fixtureEl.innerHTML = '' - const styleTest = document.createElement('style') - - styleTest.type = 'text/css' - styleTest.appendChild(document.createTextNode('body { padding-right: 7px; }')) - - document.head.appendChild(styleTest) - document.body.style.color = 'red' - - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - - modalEl.addEventListener('shown.bs.modal', () => { - modal.toggle() - }) - - modalEl.addEventListener('hidden.bs.modal', () => { - const bodyPaddingRight = document.body.style.paddingRight - - expect(bodyPaddingRight === '0px' || bodyPaddingRight === '').toEqual(true, 'body does not have inline padding set') - expect(document.body.style.color).toEqual('red', 'body still has other inline styles set') - document.head.removeChild(styleTest) - document.body.removeAttribute('style') + expect(document.body.style.overflow).toEqual(initialOverFlow) done() }) @@ -659,61 +510,6 @@ describe('Modal', () => { modal.show() }) - it('should not adjust the inline body padding when it does not overflow', done => { - fixtureEl.innerHTML = '' - - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - const originalPadding = window.getComputedStyle(document.body).paddingRight - - // Hide scrollbars to prevent the body overflowing - document.body.style.overflow = 'hidden' - document.documentElement.style.paddingRight = '0px' - - modalEl.addEventListener('shown.bs.modal', () => { - const currentPadding = window.getComputedStyle(document.body).paddingRight - - expect(currentPadding).toEqual(originalPadding, 'body padding should not be adjusted') - - // Restore scrollbars - document.body.style.overflow = 'auto' - done() - }) - - modal.show() - }) - - it('should not adjust the inline body padding when it does not overflow, even on a scaled display', done => { - fixtureEl.innerHTML = '' - - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - const originalPadding = window.getComputedStyle(document.body).paddingRight - - // Remove body margins as would be done by Bootstrap css - document.body.style.margin = '0' - - // Hide scrollbars to prevent the body overflowing - document.body.style.overflow = 'hidden' - - // Simulate a discrepancy between exact, i.e. floating point body width, and rounded body width - // as it can occur when zooming or scaling the display to something else than 100% - document.documentElement.style.paddingRight = '.48px' - - modalEl.addEventListener('shown.bs.modal', () => { - const currentPadding = window.getComputedStyle(document.body).paddingRight - - expect(currentPadding).toEqual(originalPadding, 'body padding should not be adjusted') - - // Restore overridden css - document.body.style.removeProperty('margin') - document.body.style.removeProperty('overflow') - done() - }) - - modal.show() - }) - it('should enforce focus', done => { fixtureEl.innerHTML = '' diff --git a/js/tests/unit/util/scrollbar.spec.js b/js/tests/unit/util/scrollbar.spec.js index e09a37ce70f7..b51c8a979551 100644 --- a/js/tests/unit/util/scrollbar.spec.js +++ b/js/tests/unit/util/scrollbar.spec.js @@ -4,10 +4,13 @@ import Manipulator from '../../../src/dom/manipulator' describe('ScrollBar', () => { let fixtureEl + const doc = document.documentElement const parseInt = arg => Number.parseInt(arg, 10) - const getRightPadding = el => parseInt(window.getComputedStyle(el).paddingRight) + const getPaddingX = el => parseInt(window.getComputedStyle(el).paddingRight) + const getMarginX = el => parseInt(window.getComputedStyle(el).marginRight) const getOverFlow = el => el.style.overflow const getPaddingAttr = el => Manipulator.getDataAttribute(el, 'padding-right') + const getMarginAttr = el => Manipulator.getDataAttribute(el, 'margin-right') const getOverFlowAttr = el => Manipulator.getDataAttribute(el, 'overflow') const windowCalculations = () => { return { @@ -61,8 +64,8 @@ describe('ScrollBar', () => { } }) - it('should return false if body is overflowing', () => { - document.documentElement.style.overflowY = 'hidden' + it('should return false if body is not overflowing', () => { + doc.style.overflowY = 'hidden' document.body.style.overflowY = 'hidden' fixtureEl.innerHTML = [ '
' @@ -75,7 +78,7 @@ describe('ScrollBar', () => { describe('getWidth', () => { it('should return an integer greater than zero, if body is overflowing', () => { - document.documentElement.style.overflowY = 'scroll' + doc.style.overflowY = 'scroll' document.body.style.overflowY = 'scroll' fixtureEl.innerHTML = [ '
' @@ -110,56 +113,60 @@ describe('ScrollBar', () => { '
', '' ].join('') - document.documentElement.style.overflowY = 'scroll' + doc.style.overflowY = 'scroll' const fixedEl = fixtureEl.querySelector('#fixed1') const fixedEl2 = fixtureEl.querySelector('#fixed2') - const originalPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10) - const originalPadding2 = Number.parseInt(window.getComputedStyle(fixedEl2).paddingRight, 10) + const originalPadding = getPaddingX(fixedEl) + const originalPadding2 = getPaddingX(fixedEl2) const expectedPadding = originalPadding + Scrollbar.getWidth() const expectedPadding2 = originalPadding2 + Scrollbar.getWidth() Scrollbar.hide() - let currentPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10) - let currentPadding2 = Number.parseInt(window.getComputedStyle(fixedEl2).paddingRight, 10) - expect(fixedEl.getAttribute('data-bs-padding-right')).toEqual('0px', 'original fixed element padding should be stored in data-bs-padding-right') - expect(fixedEl2.getAttribute('data-bs-padding-right')).toEqual('5px', 'original fixed element padding should be stored in data-bs-padding-right') + let currentPadding = getPaddingX(fixedEl) + let currentPadding2 = getPaddingX(fixedEl2) + expect(getPaddingAttr(fixedEl)).toEqual(`${originalPadding}px`, 'original fixed element padding should be stored in data-bs-padding-right') + expect(getPaddingAttr(fixedEl2)).toEqual(`${originalPadding2}px`, 'original fixed element padding should be stored in data-bs-padding-right') expect(currentPadding).toEqual(expectedPadding, 'fixed element padding should be adjusted while opening') expect(currentPadding2).toEqual(expectedPadding2, 'fixed element padding should be adjusted while opening') Scrollbar.reset() - currentPadding = Number.parseInt(window.getComputedStyle(fixedEl).paddingRight, 10) - currentPadding2 = Number.parseInt(window.getComputedStyle(fixedEl2).paddingRight, 10) - expect(fixedEl.getAttribute('data-bs-padding-right')).toEqual(null, 'data-bs-padding-right should be cleared after closing') - expect(fixedEl2.getAttribute('data-bs-padding-right')).toEqual(null, 'data-bs-padding-right should be cleared after closing') + currentPadding = getPaddingX(fixedEl) + currentPadding2 = getPaddingX(fixedEl2) + expect(getPaddingAttr(fixedEl)).toEqual(null, 'data-bs-padding-right should be cleared after closing') + expect(getPaddingAttr(fixedEl2)).toEqual(null, 'data-bs-padding-right should be cleared after closing') expect(currentPadding).toEqual(originalPadding, 'fixed element padding should be reset after closing') expect(currentPadding2).toEqual(originalPadding2, 'fixed element padding should be reset after closing') done() }) - it('should adjust the inline margin of sticky elements', done => { + it('should adjust the inline margin and padding of sticky elements', done => { fixtureEl.innerHTML = [ '
' + - '
', + '
', '
' ].join('') - document.documentElement.style.overflowY = 'scroll' + doc.style.overflowY = 'scroll' const stickyTopEl = fixtureEl.querySelector('.sticky-top') - const originalMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) + const originalMargin = getMarginX(stickyTopEl) + const originalPadding = getPaddingX(stickyTopEl) + const expectedMargin = originalMargin - Scrollbar.getWidth() + const expectedPadding = originalPadding + Scrollbar.getWidth() Scrollbar.hide() - let currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - expect(stickyTopEl.getAttribute('data-bs-margin-right')).toEqual('0px', 'original sticky element margin should be stored in data-bs-margin-right') - expect(currentMargin).toEqual(expectedMargin, 'sticky element margin should be adjusted while opening') + expect(getMarginAttr(stickyTopEl)).toEqual(`${originalMargin}px`, 'original sticky element margin should be stored in data-bs-margin-right') + expect(getMarginX(stickyTopEl)).toEqual(expectedMargin, 'sticky element margin should be adjusted while opening') + expect(getPaddingAttr(stickyTopEl)).toEqual(`${originalPadding}px`, 'original sticky element margin should be stored in data-bs-margin-right') + expect(getPaddingX(stickyTopEl)).toEqual(expectedPadding, 'sticky element margin should be adjusted while opening') Scrollbar.reset() - currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - - expect(stickyTopEl.getAttribute('data-bs-margin-right')).toEqual(null, 'data-bs-margin-right should be cleared after closing') - expect(currentMargin).toEqual(originalMargin, 'sticky element margin should be reset after closing') + expect(getMarginAttr(stickyTopEl)).toEqual(null, 'data-bs-margin-right should be cleared after closing') + expect(getMarginX(stickyTopEl)).toEqual(originalMargin, 'sticky element margin should be reset after closing') + expect(getPaddingAttr(stickyTopEl)).toEqual(null, 'data-bs-margin-right should be cleared after closing') + expect(getPaddingX(stickyTopEl)).toEqual(originalPadding, 'sticky element margin should be reset after closing') done() }) @@ -169,13 +176,13 @@ describe('ScrollBar', () => { ].join('') const stickyTopEl = fixtureEl.querySelector('.sticky-top') - const originalMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - const originalPadding = Number.parseInt(window.getComputedStyle(stickyTopEl).paddingRight, 10) + const originalMargin = getMarginX(stickyTopEl) + const originalPadding = getPaddingX(stickyTopEl) Scrollbar.hide() - const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - const currentPadding = Number.parseInt(window.getComputedStyle(stickyTopEl).paddingRight, 10) + const currentMargin = getMarginX(stickyTopEl) + const currentPadding = getPaddingX(stickyTopEl) expect(currentMargin).toEqual(originalMargin, 'sticky element\'s margin should not be adjusted while opening') expect(currentPadding).toEqual(originalPadding, 'sticky element\'s padding should not be adjusted while opening') @@ -183,7 +190,49 @@ describe('ScrollBar', () => { Scrollbar.reset() }) + it('should not put data-attribute if element doesn\'t have the proper style property, should just remove style property if element didn\'t had one', () => { + fixtureEl.innerHTML = [ + '
' + + '
', + '
' + ].join('') + + document.body.style.overflowY = 'scroll' + + const hasPaddingAttr = el => el.hasAttribute('data-bs-padding-right') + const hasMarginAttr = el => el.hasAttribute('data-bs-margin-right') + const stickyEl = fixtureEl.querySelector('#sticky') + const originalPadding = getPaddingX(stickyEl) + const originalMargin = getMarginX(stickyEl) + const scrollBarWidth = Scrollbar.getWidth() + + Scrollbar.hide() + + expect(getPaddingX(stickyEl)).toEqual(scrollBarWidth + originalPadding) + const expectedMargin = scrollBarWidth + originalMargin + expect(getMarginX(stickyEl)).toEqual(expectedMargin === 0 ? expectedMargin : -expectedMargin) + expect(hasMarginAttr(stickyEl)).toBeFalse() // We do not have to keep css margin + expect(hasPaddingAttr(stickyEl)).toBeFalse() // We do not have to keep css padding + + Scrollbar.reset() + + expect(getPaddingX(stickyEl)).toEqual(originalPadding) + expect(getPaddingX(stickyEl)).toEqual(originalPadding) + }) + describe('Body Handling', () => { + it('should ignore other inline styles when trying to restore body defaults ', () => { + document.body.style.color = 'red' + + const scrollBarWidth = Scrollbar.getWidth() + Scrollbar.hide() + + expect(getPaddingX(document.body)).toEqual(scrollBarWidth, 'body does not have inline padding set') + expect(document.body.style.color).toEqual('red', 'body still has other inline styles set') + + Scrollbar.reset() + }) + it('should hide scrollbar and reset it to its initial value', () => { const styleSheetPadding = '7px' fixtureEl.innerHTML = [ @@ -198,7 +247,7 @@ describe('ScrollBar', () => { const inlineStylePadding = '10px' el.style.paddingRight = inlineStylePadding - const originalPadding = getRightPadding(el) + const originalPadding = getPaddingX(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 @@ -206,7 +255,7 @@ describe('ScrollBar', () => { Scrollbar.hide() - const currentPadding = getRightPadding(el) + const currentPadding = getPaddingX(el) expect(currentPadding).toEqual(scrollBarWidth + originalPadding) expect(currentPadding).toEqual(scrollBarWidth + parseInt(inlineStylePadding)) @@ -216,7 +265,7 @@ describe('ScrollBar', () => { Scrollbar.reset() - const currentPadding1 = getRightPadding(el) + const currentPadding1 = getPaddingX(el) expect(currentPadding1).toEqual(originalPadding) expect(getPaddingAttr(el)).toEqual(null) expect(getOverFlow(el)).toEqual(originalOverFlow) @@ -233,14 +282,14 @@ describe('ScrollBar', () => { '' ].join('') const el = document.body - const originalPadding = getRightPadding(el) + const originalPadding = getPaddingX(el) const originalOverFlow = 'scroll' el.style.overflow = originalOverFlow const scrollBarWidth = Scrollbar.getWidth() Scrollbar.hide() - const currentPadding = getRightPadding(el) + const currentPadding = getPaddingX(el) expect(currentPadding).toEqual(scrollBarWidth + originalPadding) expect(currentPadding).toEqual(scrollBarWidth + parseInt(styleSheetPadding)) @@ -250,12 +299,46 @@ describe('ScrollBar', () => { Scrollbar.reset() - const currentPadding1 = getRightPadding(el) + const currentPadding1 = getPaddingX(el) expect(currentPadding1).toEqual(originalPadding) expect(getPaddingAttr(el)).toEqual(null) expect(getOverFlow(el)).toEqual(originalOverFlow) expect(getOverFlowAttr(el)).toEqual(null) }) + + it('should not adjust the inline body padding when it does not overflow', () => { + const originalPadding = getPaddingX(document.body) + + // Hide scrollbars to prevent the body overflowing + doc.style.overflowY = 'hidden' + doc.style.paddingRight = '0px' + + Scrollbar.hide() + const currentPadding = getPaddingX(document.body) + + expect(currentPadding).toEqual(originalPadding, 'body padding should not be adjusted') + Scrollbar.reset() + }) + + it('should not adjust the inline body padding when it does not overflow, even on a scaled display', () => { + const originalPadding = getPaddingX(document.body) + // Remove body margins as would be done by Bootstrap css + document.body.style.margin = '0' + + // Hide scrollbars to prevent the body overflowing + doc.style.overflowY = 'hidden' + + // Simulate a discrepancy between exact, i.e. floating point body width, and rounded body width + // as it can occur when zooming or scaling the display to something else than 100% + doc.style.paddingRight = '.48px' + Scrollbar.hide() + + const currentPadding = getPaddingX(document.body) + + expect(currentPadding).toEqual(originalPadding, 'body padding should not be adjusted') + + Scrollbar.reset() + }) }) }) })