diff --git a/js/src/modal.js b/js/src/modal.js index b2a2e80ebc04..dea90ec0a758 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -18,6 +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 BaseComponent from './base-component' /** @@ -57,7 +58,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_SCROLLBAR_MEASURER = 'modal-scrollbar-measure' const CLASS_NAME_BACKDROP = 'modal-backdrop' const CLASS_NAME_OPEN = 'modal-open' const CLASS_NAME_FADE = 'fade' @@ -68,8 +68,6 @@ const SELECTOR_DIALOG = '.modal-dialog' const SELECTOR_MODAL_BODY = '.modal-body' const SELECTOR_DATA_TOGGLE = '[data-bs-toggle="modal"]' const SELECTOR_DATA_DISMISS = '[data-bs-dismiss="modal"]' -const SELECTOR_FIXED_CONTENT = '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top' -const SELECTOR_STICKY_CONTENT = '.sticky-top' /** * ------------------------------------------------------------------------ @@ -85,10 +83,8 @@ class Modal extends BaseComponent { this._dialog = SelectorEngine.findOne(SELECTOR_DIALOG, this._element) this._backdrop = null this._isShown = false - this._isBodyOverflowing = false this._ignoreBackdropClick = false this._isTransitioning = false - this._scrollbarWidth = 0 } // Getters @@ -126,8 +122,9 @@ class Modal extends BaseComponent { this._isShown = true - this._checkScrollbar() - this._setScrollbar() + scrollBarHide() + + document.body.classList.add(CLASS_NAME_OPEN) this._adjustDialog() @@ -206,10 +203,8 @@ class Modal extends BaseComponent { this._dialog = null this._backdrop = null this._isShown = null - this._isBodyOverflowing = null this._ignoreBackdropClick = null this._isTransitioning = null - this._scrollbarWidth = null } handleUpdate() { @@ -321,7 +316,7 @@ class Modal extends BaseComponent { this._showBackdrop(() => { document.body.classList.remove(CLASS_NAME_OPEN) this._resetAdjustments() - this._resetScrollbar() + scrollBarReset() EventHandler.trigger(this._element, EVENT_HIDDEN) }) } @@ -433,13 +428,15 @@ class Modal extends BaseComponent { _adjustDialog() { const isModalOverflowing = this._element.scrollHeight > document.documentElement.clientHeight + const scrollbarWidth = getScrollBarWidth() + const isBodyOverflowing = scrollbarWidth > 0 - if ((!this._isBodyOverflowing && isModalOverflowing && !isRTL()) || (this._isBodyOverflowing && !isModalOverflowing && isRTL())) { - this._element.style.paddingLeft = `${this._scrollbarWidth}px` + if ((!isBodyOverflowing && isModalOverflowing && !isRTL()) || (isBodyOverflowing && !isModalOverflowing && isRTL())) { + this._element.style.paddingLeft = `${scrollbarWidth}px` } - if ((this._isBodyOverflowing && !isModalOverflowing && !isRTL()) || (!this._isBodyOverflowing && isModalOverflowing && isRTL())) { - this._element.style.paddingRight = `${this._scrollbarWidth}px` + if ((isBodyOverflowing && !isModalOverflowing && !isRTL()) || (!isBodyOverflowing && isModalOverflowing && isRTL())) { + this._element.style.paddingRight = `${scrollbarWidth}px` } } @@ -448,63 +445,6 @@ class Modal extends BaseComponent { this._element.style.paddingRight = '' } - _checkScrollbar() { - const rect = document.body.getBoundingClientRect() - this._isBodyOverflowing = Math.round(rect.left + rect.right) < window.innerWidth - this._scrollbarWidth = this._getScrollbarWidth() - } - - _setScrollbar() { - if (this._isBodyOverflowing) { - this._setElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight', calculatedValue => calculatedValue + this._scrollbarWidth) - this._setElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight', calculatedValue => calculatedValue - this._scrollbarWidth) - this._setElementAttributes('body', 'paddingRight', calculatedValue => calculatedValue + this._scrollbarWidth) - } - - document.body.classList.add(CLASS_NAME_OPEN) - } - - _setElementAttributes(selector, styleProp, callback) { - SelectorEngine.find(selector) - .forEach(element => { - if (element !== document.body && window.innerWidth > element.clientWidth + this._scrollbarWidth) { - return - } - - const actualValue = element.style[styleProp] - const calculatedValue = window.getComputedStyle(element)[styleProp] - Manipulator.setDataAttribute(element, styleProp, actualValue) - element.style[styleProp] = `${callback(Number.parseFloat(calculatedValue))}px` - }) - } - - _resetScrollbar() { - this._resetElementAttributes(SELECTOR_FIXED_CONTENT, 'paddingRight') - this._resetElementAttributes(SELECTOR_STICKY_CONTENT, 'marginRight') - this._resetElementAttributes('body', 'paddingRight') - } - - _resetElementAttributes(selector, styleProp) { - SelectorEngine.find(selector).forEach(element => { - const value = Manipulator.getDataAttribute(element, styleProp) - if (typeof value === 'undefined' && element === document.body) { - element.style[styleProp] = '' - } else { - Manipulator.removeDataAttribute(element, styleProp) - element.style[styleProp] = value - } - }) - } - - _getScrollbarWidth() { // thx d.walsh - const scrollDiv = document.createElement('div') - scrollDiv.className = CLASS_NAME_SCROLLBAR_MEASURER - document.body.appendChild(scrollDiv) - const scrollbarWidth = scrollDiv.getBoundingClientRect().width - scrollDiv.clientWidth - document.body.removeChild(scrollDiv) - return scrollbarWidth - } - // Static static jQueryInterface(config, relatedTarget) { diff --git a/js/src/util/scrollbar.js b/js/src/util/scrollbar.js index e63a66bf218b..31b614375603 100644 --- a/js/src/util/scrollbar.js +++ b/js/src/util/scrollbar.js @@ -8,7 +8,7 @@ import SelectorEngine from '../dom/selector-engine' import Manipulator from '../dom/manipulator' -const SELECTOR_FIXED_CONTENT = '.fixed-top, .fixed-bottom, .is-fixed' +const SELECTOR_FIXED_CONTENT = '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top' const SELECTOR_STICKY_CONTENT = '.sticky-top' const getWidth = () => { @@ -19,6 +19,7 @@ const getWidth = () => { const hide = (width = getWidth()) => { document.body.style.overflow = 'hidden' + // 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) @@ -49,7 +50,7 @@ const reset = () => { const _resetElementAttributes = (selector, styleProp) => { SelectorEngine.find(selector).forEach(element => { const value = Manipulator.getDataAttribute(element, styleProp) - if (typeof value === 'undefined' && element === document.body) { + if (typeof value === 'undefined') { element.style.removeProperty(styleProp) } else { Manipulator.removeDataAttribute(element, styleProp) diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index f10a932d2a51..99ebbe4b37b5 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -1,27 +1,15 @@ import Modal from '../../src/modal' 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' describe('Modal', () => { let fixtureEl - let style beforeAll(() => { fixtureEl = getFixture() - - // Enable the scrollbar measurer - const css = '.modal-scrollbar-measure { position: absolute; top: -9999px; width: 50px; height: 50px; overflow: scroll; }' - - style = document.createElement('style') - style.type = 'text/css' - style.appendChild(document.createTextNode(css)) - - document.head.appendChild(style) - - // Simulate scrollbars - document.documentElement.style.paddingRight = '16px' }) afterEach(() => { @@ -36,12 +24,11 @@ describe('Modal', () => { document.body.removeChild(backdrop) }) - document.body.style.paddingRight = '0px' + document.body.style.removeProperty('paddingRight') }) afterAll(() => { - document.head.removeChild(style) - document.documentElement.style.paddingRight = '0px' + document.documentElement.style.removeProperty('paddingRight') }) describe('VERSION', () => { @@ -79,6 +66,7 @@ describe('Modal', () => { 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' @@ -93,6 +81,7 @@ describe('Modal', () => { modalEl.addEventListener('hidden.bs.modal', () => { expect(document.body.getAttribute('data-bs-padding-right')).toBeNull() expect().nothing() + document.documentElement.style.overflowY = 'auto' done() }) @@ -105,13 +94,14 @@ describe('Modal', () => { '' ].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) modalEl.addEventListener('shown.bs.modal', () => { - const expectedPadding = originalPadding + modal._getScrollbarWidth() + const expectedPadding = originalPadding + getScrollBarWidth() const currentPadding = Number.parseInt(window.getComputedStyle(modalEl).paddingRight, 10) expect(fixedEl.getAttribute('data-bs-padding-right')).toEqual('0px', 'original fixed element padding should be stored in data-bs-padding-right') @@ -124,6 +114,7 @@ describe('Modal', () => { expect(fixedEl.getAttribute('data-bs-padding-right')).toEqual(null, '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() }) @@ -136,13 +127,15 @@ describe('Modal', () => { '' ].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) modalEl.addEventListener('shown.bs.modal', () => { - const expectedMargin = originalMargin - modal._getScrollbarWidth() + const expectedMargin = originalMargin - getScrollBarWidth() const 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') @@ -155,6 +148,8 @@ describe('Modal', () => { 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') + + document.documentElement.style.overflowY = 'auto' done() }) @@ -238,27 +233,6 @@ describe('Modal', () => { modal.toggle() }) - - it('should properly restore non-pixel inline body padding after closing', done => { - fixtureEl.innerHTML = '' - - document.body.style.paddingRight = '5%' - - const modalEl = fixtureEl.querySelector('.modal') - const modal = new Modal(modalEl) - - modalEl.addEventListener('shown.bs.modal', () => { - modal.toggle() - }) - - modalEl.addEventListener('hidden.bs.modal', () => { - expect(document.body.style.paddingRight).toEqual('5%') - document.body.removeAttribute('style') - done() - }) - - modal.toggle() - }) }) describe('show', () => { diff --git a/scss/_modal.scss b/scss/_modal.scss index 6dd4dd32941f..513898644d26 100644 --- a/scss/_modal.scss +++ b/scss/_modal.scss @@ -159,15 +159,6 @@ } } -// Measure scrollbar width for padding body during modal show/hide -.modal-scrollbar-measure { - position: absolute; - top: -9999px; - width: 50px; - height: 50px; - overflow: scroll; -} - // Scale up the modal @include media-breakpoint-up(sm) { // Automatically set modal's width for larger viewports