From 1cc295e2b442a67df09d6e01ae196088ad7b4b15 Mon Sep 17 00:00:00 2001 From: GeoSot Date: Sat, 17 Apr 2021 04:05:25 +0300 Subject: [PATCH] Scrollbar.js Sanitizations * Remove `.modal-open` class as it is already handled by scrollbar.js * _modal.scss make modal `overflowY: auto` by default (was on ``.modal-open`, which is the same) * scrollbar.js: avoid to create data-bs attributes for null values & test * Revisit modal, offcanvas, scrollbar tests * set the base to support different rootElement (than body) --- js/src/modal.js | 13 +- js/src/offcanvas.js | 6 +- js/src/util/scrollbar.js | 124 +++++---- js/tests/unit/modal.spec.js | 217 +-------------- js/tests/unit/offcanvas.spec.js | 19 +- js/tests/unit/util/scrollbar.spec.js | 324 +++++++++++++++------- scss/_modal.scss | 15 +- site/content/docs/5.0/components/modal.md | 2 +- 8 files changed, 332 insertions(+), 388 deletions(-) diff --git a/js/src/modal.js b/js/src/modal.js index 058d4a36b28f..60462ecf7e9c 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -18,7 +18,7 @@ import { import EventHandler from './dom/event-handler' import Manipulator from './dom/manipulator' import SelectorEngine from './dom/selector-engine' -import { getWidth as getScrollBarWidth, hide as scrollBarHide, reset as scrollBarReset } from './util/scrollbar' +import ScrollBarHelper from './util/scrollbar' import BaseComponent from './base-component' import Backdrop from './util/backdrop' @@ -59,7 +59,6 @@ const EVENT_MOUSEUP_DISMISS = `mouseup.dismiss${EVENT_KEY}` const EVENT_MOUSEDOWN_DISMISS = `mousedown.dismiss${EVENT_KEY}` const EVENT_CLICK_DATA_API = `click${EVENT_KEY}${DATA_API_KEY}` -const CLASS_NAME_OPEN = 'modal-open' const CLASS_NAME_FADE = 'fade' const CLASS_NAME_SHOW = 'show' const CLASS_NAME_STATIC = 'modal-static' @@ -85,6 +84,7 @@ class Modal extends BaseComponent { this._isShown = false this._ignoreBackdropClick = false this._isTransitioning = false + this._scrollBar = new ScrollBarHelper() } // Getters @@ -122,9 +122,7 @@ class Modal extends BaseComponent { this._isShown = true - scrollBarHide() - - document.body.classList.add(CLASS_NAME_OPEN) + this._scrollBar.hide() this._adjustDialog() @@ -301,9 +299,8 @@ class Modal extends BaseComponent { this._element.removeAttribute('role') this._isTransitioning = false this._backdrop.hide(() => { - document.body.classList.remove(CLASS_NAME_OPEN) this._resetAdjustments() - scrollBarReset() + this._scrollBar.reset() EventHandler.trigger(this._element, EVENT_HIDDEN) }) } @@ -367,7 +364,7 @@ class Modal extends BaseComponent { _adjustDialog() { const isModalOverflowing = this._element.scrollHeight > document.documentElement.clientHeight - const scrollbarWidth = getScrollBarWidth() + const scrollbarWidth = this._scrollBar.getWidth() const isBodyOverflowing = scrollbarWidth > 0 if ((!isBodyOverflowing && isModalOverflowing && !isRTL()) || (isBodyOverflowing && !isModalOverflowing && isRTL())) { diff --git a/js/src/offcanvas.js b/js/src/offcanvas.js index 65d1e6ba752a..900487c2ddcf 100644 --- a/js/src/offcanvas.js +++ b/js/src/offcanvas.js @@ -12,7 +12,7 @@ import { isVisible, typeCheckConfig } from './util/index' -import { hide as scrollBarHide, reset as scrollBarReset } from './util/scrollbar' +import ScrollBarHelper from './util/scrollbar' import Data from './dom/data' import EventHandler from './dom/event-handler' import BaseComponent from './base-component' @@ -109,7 +109,7 @@ class Offcanvas extends BaseComponent { this._backdrop.show() if (!this._config.scroll) { - scrollBarHide() + new ScrollBarHelper().hide() this._enforceFocusOnElement(this._element) } @@ -149,7 +149,7 @@ class Offcanvas extends BaseComponent { this._element.style.visibility = 'hidden' if (!this._config.scroll) { - scrollBarReset() + new ScrollBarHelper().reset() } EventHandler.trigger(this._element, EVENT_HIDDEN) diff --git a/js/src/util/scrollbar.js b/js/src/util/scrollbar.js index 87ea1390ad50..41bd222ee599 100644 --- a/js/src/util/scrollbar.js +++ b/js/src/util/scrollbar.js @@ -7,75 +7,99 @@ import SelectorEngine from '../dom/selector-engine' import Manipulator from '../dom/manipulator' +import { isElement } from './index' const SELECTOR_FIXED_CONTENT = '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top' const SELECTOR_STICKY_CONTENT = '.sticky-top' -const getWidth = () => { - // https://developer.mozilla.org/en-US/docs/Web/API/Window/innerWidth#usage_notes - const documentWidth = document.documentElement.clientWidth - return Math.abs(window.innerWidth - documentWidth) -} +class ScrollBarHelper { + constructor(element = document.documentElement) { + this._isBody = [document.body, document.documentElement].includes(element) -const hide = (width = getWidth()) => { - _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) -} + this._element = this._isBody ? document.body : element + } -const _disableOverFlow = () => { - const actualValue = document.body.style.overflow - if (actualValue) { - Manipulator.setDataAttribute(document.body, 'overflow', actualValue) + getWidth() { + if (this._isBody) { + // https://developer.mozilla.org/en-US/docs/Web/API/Window/innerWidth#usage_notes + const documentWidth = document.documentElement.clientWidth + return Math.abs(window.innerWidth - documentWidth) + } + + return Math.abs(this._element.offsetWidth - this._element.clientWidth) } - document.body.style.overflow = 'hidden' -} + hide() { + const width = this.getWidth() + this._disableOverFlow() + // give padding to element to balances the hidden scrollbar width + this._setElementAttributes(this._element, 'paddingRight', calculatedValue => calculatedValue + width) + // trick: We adjust positive paddingRight and negative marginRight to sticky-top elements, to keep shown fullwidth + this._setElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight', calculatedValue => calculatedValue + width) + this._setElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight', calculatedValue => calculatedValue - width) + } -const _setElementAttributes = (selector, styleProp, callback) => { - const scrollbarWidth = getWidth() - SelectorEngine.find(selector) - .forEach(element => { - if (element !== document.body && window.innerWidth > element.clientWidth + scrollbarWidth) { + _disableOverFlow() { + const actualValue = this._element.style.overflow + if (actualValue) { + Manipulator.setDataAttribute(this._element, 'overflow', actualValue) + } + + this._element.style.overflow = 'hidden' + } + + _setElementAttributes(selector, styleProp, callback) { + const scrollbarWidth = this.getWidth() + const manipulationCallBack = element => { + if (element !== this._element && window.innerWidth > element.clientWidth + scrollbarWidth) { return } 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` - }) -} + } -const reset = () => { - _resetElementAttributes('body', 'overflow') - _resetElementAttributes('body', 'paddingRight') - _resetElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight') - _resetElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight') -} + this._applyManipulationCallback(selector, manipulationCallBack) + } + + reset() { + this._resetElementAttributes(this._element, 'overflow') + this._resetElementAttributes(this._element, 'paddingRight') + this._resetElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight') + this._resetElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight') + } + + _resetElementAttributes(selector, styleProp) { + const manipulationCallBack = element => { + const value = Manipulator.getDataAttribute(element, styleProp) + if (typeof value === 'undefined') { + element.style.removeProperty(styleProp) + } else { + Manipulator.removeDataAttribute(element, styleProp) + element.style[styleProp] = value + } + } + + this._applyManipulationCallback(selector, manipulationCallBack) + } -const _resetElementAttributes = (selector, styleProp) => { - SelectorEngine.find(selector).forEach(element => { - const value = Manipulator.getDataAttribute(element, styleProp) - if (typeof value === 'undefined') { - element.style.removeProperty(styleProp) + _applyManipulationCallback(selector, callBack) { + if (isElement(selector)) { + callBack(selector) } else { - Manipulator.removeDataAttribute(element, styleProp) - element.style[styleProp] = value + SelectorEngine.find(selector, this._element).forEach(callBack) } - }) -} + } -const isBodyOverflowing = () => { - return getWidth() > 0 + isOverflowing(el = document.body) { + const clientHeight = this._isBody ? Math.min(document.body.clientHeight, document.documentElement.clientHeight, window.innerHeight) : this._element.clientHeight + return el.scrollHeight > clientHeight + } } -export { - getWidth, - hide, - isBodyOverflowing, - reset -} +export default ScrollBarHelper diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index 79f3c4845bc3..71edd4dc8c4c 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -1,6 +1,6 @@ import Modal from '../../src/modal' import EventHandler from '../../src/dom/event-handler' -import { getWidth as getScrollBarWidth } from '../../src/util/scrollbar' +import ScrollBarHelper from '../../src/util/scrollbar' /** Test helpers */ import { clearBodyAndDocument, clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture' @@ -59,173 +59,23 @@ describe('Modal', () => { }) describe('toggle', () => { - it('should toggle a modal', done => { - fixtureEl.innerHTML = '
' - - document.documentElement.style.overflowY = 'scroll' - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - const originalPadding = '0px' - - 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') - 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 => { + it('should call ScrollBarHelper to handle scrollBar on body', 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) - + spyOn(ScrollBarHelper.prototype, 'hide').and.callThrough() + spyOn(ScrollBarHelper.prototype, 'reset').and.callThrough() const modalEl = fixtureEl.querySelector('.modal') const modal = new Modal(modalEl) modalEl.addEventListener('shown.bs.modal', () => { + expect(ScrollBarHelper.prototype.hide).toHaveBeenCalled() 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(ScrollBarHelper.prototype.reset).toHaveBeenCalled() done() }) @@ -659,61 +509,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/offcanvas.spec.js b/js/tests/unit/offcanvas.spec.js index dfb6c429e2e5..51f81ec2b432 100644 --- a/js/tests/unit/offcanvas.spec.js +++ b/js/tests/unit/offcanvas.spec.js @@ -4,6 +4,7 @@ import EventHandler from '../../src/dom/event-handler' /** Test helpers */ import { clearBodyAndDocument, clearFixture, createEvent, getFixture, jQueryMock } from '../helpers/fixture' import { isVisible } from '../../src/util' +import ScrollBarHelper from '../../src/util/scrollbar' describe('Offcanvas', () => { let fixtureEl @@ -14,7 +15,6 @@ describe('Offcanvas', () => { afterEach(() => { clearFixture() - document.body.classList.remove('offcanvas-open') clearBodyAndDocument() }) @@ -159,36 +159,37 @@ describe('Offcanvas', () => { it('if scroll is enabled, should allow body to scroll while offcanvas is open', done => { fixtureEl.innerHTML = '
' + spyOn(ScrollBarHelper.prototype, 'hide').and.callThrough() + spyOn(ScrollBarHelper.prototype, 'reset').and.callThrough() const offCanvasEl = fixtureEl.querySelector('.offcanvas') const offCanvas = new Offcanvas(offCanvasEl, { scroll: true }) - const initialOverFlow = document.body.style.overflow offCanvasEl.addEventListener('shown.bs.offcanvas', () => { - expect(document.body.style.overflow).toEqual(initialOverFlow) - + expect(ScrollBarHelper.prototype.hide).not.toHaveBeenCalled() offCanvas.hide() }) offCanvasEl.addEventListener('hidden.bs.offcanvas', () => { - expect(document.body.style.overflow).toEqual(initialOverFlow) + expect(ScrollBarHelper.prototype.reset).not.toHaveBeenCalled() done() }) offCanvas.show() }) - it('if scroll is disabled, should not allow body to scroll while offcanvas is open', done => { + it('if scroll is disabled, should call ScrollBarHelper to handle scrollBar on body', done => { fixtureEl.innerHTML = '
' + spyOn(ScrollBarHelper.prototype, 'hide').and.callThrough() + spyOn(ScrollBarHelper.prototype, 'reset').and.callThrough() const offCanvasEl = fixtureEl.querySelector('.offcanvas') const offCanvas = new Offcanvas(offCanvasEl, { scroll: false }) const initialOverFlow = document.body.style.overflow offCanvasEl.addEventListener('shown.bs.offcanvas', () => { - expect(document.body.style.overflow).toEqual('hidden') - + expect(ScrollBarHelper.prototype.hide).toHaveBeenCalled() offCanvas.hide() }) offCanvasEl.addEventListener('hidden.bs.offcanvas', () => { - expect(document.body.style.overflow).toEqual(initialOverFlow) + expect(ScrollBarHelper.prototype.reset).toHaveBeenCalled() done() }) offCanvas.show() diff --git a/js/tests/unit/util/scrollbar.spec.js b/js/tests/unit/util/scrollbar.spec.js index e09a37ce70f7..0a471a268490 100644 --- a/js/tests/unit/util/scrollbar.spec.js +++ b/js/tests/unit/util/scrollbar.spec.js @@ -1,13 +1,16 @@ -import * as Scrollbar from '../../../src/util/scrollbar' import { clearBodyAndDocument, clearFixture, getFixture } from '../../helpers/fixture' import Manipulator from '../../../src/dom/manipulator' +import ScrollBarHelper from '../../../src/util/scrollbar' 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 { @@ -45,60 +48,118 @@ describe('ScrollBar', () => { clearBodyAndDocument() }) - describe('isBodyOverflowing', () => { - it('should return true if body is overflowing', () => { - document.documentElement.style.overflowY = 'scroll' - document.body.style.overflowY = 'scroll' - fixtureEl.innerHTML = [ - '
' - ].join('') - const result = Scrollbar.isBodyOverflowing() + describe('isOverflowing', () => { + describe('Body', () => { + it('should return true if body is overflowing', () => { + fixtureEl.innerHTML = [ + '
text
' + ].join('') + const result = new ScrollBarHelper().isOverflowing() - if (isScrollBarHidden()) { - expect(result).toEqual(false) - } else { expect(result).toEqual(true) - } + }) + + it('should return false if body is not overflowing', () => { + doc.style.height = '100vh' + document.body.style.height = '100vh' + fixtureEl.innerHTML = [ + '
' + ].join('') + const scrollBar = new ScrollBarHelper() + const result = scrollBar.isOverflowing() + + expect(result).toEqual(false) + const result2 = scrollBar.isOverflowing(fixtureEl.querySelector('div')) + expect(result2).toEqual(false) + }) }) + describe('Element', () => { + it('should return true if wrapper element is overflowing', () => { + fixtureEl.innerHTML = [ + '
' + + '
' + + '
' + ].join('') + const wrapper = fixtureEl.querySelector('.wrapper') + const innerElement = fixtureEl.querySelector('.inner') + const result = new ScrollBarHelper(wrapper).isOverflowing(innerElement) - it('should return false if body is overflowing', () => { - document.documentElement.style.overflowY = 'hidden' - document.body.style.overflowY = 'hidden' - fixtureEl.innerHTML = [ - '
' - ].join('') - const result = Scrollbar.isBodyOverflowing() + expect(result).toEqual(true) + }) - expect(result).toEqual(false) + it('should return false if wrapper element is not overflowing', () => { + fixtureEl.innerHTML = [ + '
' + + '
' + + '
' + ].join('') + const wrapper = fixtureEl.querySelector('.wrapper') + const innerElement = fixtureEl.querySelector('.inner') + const result = new ScrollBarHelper(wrapper).isOverflowing(innerElement) + + expect(result).toEqual(false) + }) }) }) describe('getWidth', () => { - it('should return an integer greater than zero, if body is overflowing', () => { - document.documentElement.style.overflowY = 'scroll' - document.body.style.overflowY = 'scroll' - fixtureEl.innerHTML = [ - '
' - ].join('') - const result = Scrollbar.getWidth() + describe('Body', () => { + it('should return an integer greater than zero, if body is overflowing', () => { + document.documentElement.style.overflowY = 'scroll' + document.body.style.overflowY = 'scroll' + fixtureEl.innerHTML = [ + '
' + ].join('') + const result = new ScrollBarHelper().getWidth() + + if (isScrollBarHidden()) { + expect(result).toBe(0) + } else { + expect(result).toBeGreaterThan(1) + } + }) - if (isScrollBarHidden()) { - expect(result).toBe(0) - } else { - expect(result).toBeGreaterThan(1) - } + it('should return 0 if body is not overflowing', () => { + document.documentElement.style.overflowY = 'hidden' + document.body.style.overflowY = 'hidden' + fixtureEl.innerHTML = [ + '
' + ].join('') + + const result = new ScrollBarHelper().getWidth() + + expect(result).toEqual(0) + }) }) - it('should return 0 if body is not overflowing', () => { - document.documentElement.style.overflowY = 'hidden' - document.body.style.overflowY = 'hidden' - fixtureEl.innerHTML = [ - '
' - ].join('') + describe('Element', () => { + it('should return an integer greater than zero, if wrapper element is overflowing', () => { + fixtureEl.innerHTML = [ + '
' + + '
' + + '
' + ].join('') + const wrapper = fixtureEl.querySelector('.wrapper') + const result = new ScrollBarHelper(wrapper).getWidth() + + if (isScrollBarHidden()) { + expect(result).toBe(0) + } else { + expect(result).toBeGreaterThan(1) + } + }) - const result = Scrollbar.getWidth() + it('should return 0 if wrapper element is not overflowing', () => { + fixtureEl.innerHTML = [ + '
' + + '
' + + '
' + ].join('') + const wrapper = fixtureEl.querySelector('.wrapper') + const result = new ScrollBarHelper(wrapper).getWidth() - expect(result).toEqual(0) + expect(result).toEqual(0) + }) }) }) @@ -106,7 +167,7 @@ describe('ScrollBar', () => { it('should adjust the inline padding of fixed elements which are full-width', done => { fixtureEl.innerHTML = [ '
' + - '
', + '
', '
', '
' ].join('') @@ -114,52 +175,57 @@ describe('ScrollBar', () => { 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 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') + const originalPadding = getPaddingX(fixedEl) + const originalPadding2 = getPaddingX(fixedEl2) + const scrollBar = new ScrollBarHelper() + const expectedPadding = originalPadding + scrollBar.getWidth() + const expectedPadding2 = originalPadding2 + scrollBar.getWidth() + + scrollBar.hide() + + 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') + scrollBar.reset() + 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 expectedMargin = originalMargin - 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') - - 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') + const originalMargin = getMarginX(stickyTopEl) + const originalPadding = getPaddingX(stickyTopEl) + const scrollBar = new ScrollBarHelper() + const expectedMargin = originalMargin - scrollBar.getWidth() + const expectedPadding = originalPadding + scrollBar.getWidth() + scrollBar.hide() + + 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() + 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,18 +235,50 @@ 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) + const scrollBar = new ScrollBarHelper() - Scrollbar.hide() + 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') - Scrollbar.reset() + 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 scrollBar = new ScrollBarHelper() + const scrollBarWidth = scrollBar.getWidth() + + scrollBar.hide() + + expect(getPaddingX(stickyEl)).toEqual(scrollBarWidth + originalPadding) + const expectedMargin = scrollBarWidth + originalMargin + expect(getMarginX(stickyEl)).toEqual(Math.abs(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', () => { @@ -198,15 +296,16 @@ 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 - const scrollBarWidth = Scrollbar.getWidth() + const scrollBar = new ScrollBarHelper() + const scrollBarWidth = scrollBar.getWidth() - Scrollbar.hide() + scrollBar.hide() - const currentPadding = getRightPadding(el) + const currentPadding = getPaddingX(el) expect(currentPadding).toEqual(scrollBarWidth + originalPadding) expect(currentPadding).toEqual(scrollBarWidth + parseInt(inlineStylePadding)) @@ -214,9 +313,9 @@ describe('ScrollBar', () => { expect(getOverFlow(el)).toEqual('hidden') expect(getOverFlowAttr(el)).toEqual(originalOverFlow) - Scrollbar.reset() + 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 +332,15 @@ 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() + const scrollBar = new ScrollBarHelper() + const scrollBarWidth = scrollBar.getWidth() - Scrollbar.hide() + scrollBar.hide() - const currentPadding = getRightPadding(el) + const currentPadding = getPaddingX(el) expect(currentPadding).toEqual(scrollBarWidth + originalPadding) expect(currentPadding).toEqual(scrollBarWidth + parseInt(styleSheetPadding)) @@ -248,14 +348,52 @@ describe('ScrollBar', () => { expect(getOverFlow(el)).toEqual('hidden') expect(getOverFlowAttr(el)).toEqual(originalOverFlow) - Scrollbar.reset() + 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 scrollBar = new ScrollBarHelper() + 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 scrollBar = new ScrollBarHelper() + + const originalPadding = getPaddingX(document.body) + const doc = document.documentElement + // 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() + }) }) }) }) diff --git a/scss/_modal.scss b/scss/_modal.scss index 513898644d26..3b5ba6b7fef6 100644 --- a/scss/_modal.scss +++ b/scss/_modal.scss @@ -1,19 +1,7 @@ -// .modal-open - body class for killing the scroll // .modal - container to scroll within // .modal-dialog - positioning shell for the actual modal // .modal-content - actual modal w/ bg and corners and stuff - -.modal-open { - // Kill the scroll on the body - overflow: hidden; - - .modal { - overflow-x: hidden; - overflow-y: auto; - } -} - // Container that the modal scrolls within .modal { position: fixed; @@ -23,7 +11,8 @@ display: none; width: 100%; height: 100%; - overflow: hidden; + overflow-x: hidden; + overflow-y: auto; // Prevent Chrome on Windows from adding a focus outline. For details, see // https://github.com/twbs/bootstrap/pull/10951. outline: 0; diff --git a/site/content/docs/5.0/components/modal.md b/site/content/docs/5.0/components/modal.md index 9917656a57cb..bb7aebe0a4f7 100644 --- a/site/content/docs/5.0/components/modal.md +++ b/site/content/docs/5.0/components/modal.md @@ -827,7 +827,7 @@ Another override is the option to pop up a modal that covers the user viewport, ## Usage -The modal plugin toggles your hidden content on demand, via data attributes or JavaScript. It also adds `.modal-open` to the `` to override default scrolling behavior and generates a `.modal-backdrop` to provide a click area for dismissing shown modals when clicking outside the modal. +The modal plugin toggles your hidden content on demand, via data attributes or JavaScript. It overrides the `` default scrolling behavior and generates a `.modal-backdrop` to provide a click area for dismissing shown modals when clicking outside the modal. ### Via data attributes