From 2d6021bd65929684f350eb0e8ac304377fa21249 Mon Sep 17 00:00:00 2001 From: Muhammadamin Date: Mon, 20 Apr 2020 21:16:06 +0300 Subject: [PATCH 1/7] modal: don't add margin & padding when sticky is not full width --- js/src/modal.js | 8 ++++++++ js/tests/unit/modal.spec.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/js/src/modal.js b/js/src/modal.js index 87c22943a..1b8363c01 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -462,6 +462,10 @@ class Modal extends BaseComponent { // Adjust fixed content padding SelectorEngine.find(SELECTOR_FIXED_CONTENT) .forEach(element => { + if (window.innerWidth > element.clientWidth + this._scrollbarWidth) { + return + } + const actualPadding = element.style.paddingRight const calculatedPadding = window.getComputedStyle(element)['padding-right'] Manipulator.setDataAttribute(element, 'padding-right', actualPadding) @@ -471,6 +475,10 @@ class Modal extends BaseComponent { // Adjust sticky content margin SelectorEngine.find(SELECTOR_STICKY_CONTENT) .forEach(element => { + if (window.innerWidth > element.clientWidth + this._scrollbarWidth) { + return + } + const actualMargin = element.style.marginRight const calculatedMargin = window.getComputedStyle(element)['margin-right'] Manipulator.setDataAttribute(element, 'margin-right', actualMargin) diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index f645e9892..7cd74fdc2 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -142,6 +142,36 @@ describe('Modal', () => { modal.toggle() }) + it('should not adjust the inline margin of sticky elements when element do not have full width', done => { + fixtureEl.innerHTML = [ + '
', + '
' + ].join('') + + const stickyTopEl = fixtureEl.querySelector('.sticky-top') + const originalMargin = parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) + const modalEl = fixtureEl.querySelector('.modal') + const modal = new Modal(modalEl) + + modalEl.addEventListener('shown.bs.modal', () => { + const expectedMargin = 0 + const currentMargin = parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) + + expect(currentMargin).toEqual(expectedMargin, 'sticky element margin should not be adjusted while opening') + modal.toggle() + }) + + modalEl.addEventListener('hidden.bs.modal', () => { + const currentMargin = parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) + + expect(stickyTopEl.getAttribute('data-margin-right')).toEqual(null, 'data-margin-right should be cleared after closing') + expect(currentMargin).toEqual(originalMargin, 'sticky element margin should be reset after closing') + done() + }) + + modal.toggle() + }) + it('should ignore values set via CSS when trying to restore body padding after closing', done => { fixtureEl.innerHTML = '
' const styleTest = document.createElement('style') From c19d06acbf4c4776ebd1a1357de324c27de8a4da Mon Sep 17 00:00:00 2001 From: XhmikosR Date: Fri, 20 Nov 2020 20:50:40 +0200 Subject: [PATCH 2/7] Update modal.spec.js --- js/tests/unit/modal.spec.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index 7cd74fdc2..34e1ac2d8 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -149,20 +149,19 @@ describe('Modal', () => { ].join('') const stickyTopEl = fixtureEl.querySelector('.sticky-top') - const originalMargin = parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) + 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 = 0 - const currentMargin = parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) + const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - expect(currentMargin).toEqual(expectedMargin, 'sticky element margin should not be adjusted while opening') + expect(currentMargin).toEqual(originalMargin, 'sticky element margin should not be adjusted while opening') modal.toggle() }) modalEl.addEventListener('hidden.bs.modal', () => { - const currentMargin = parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) + const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) expect(stickyTopEl.getAttribute('data-margin-right')).toEqual(null, 'data-margin-right should be cleared after closing') expect(currentMargin).toEqual(originalMargin, 'sticky element margin should be reset after closing') From ee959189a7b8a1b088c4c7b663cf896fcfb7571f Mon Sep 17 00:00:00 2001 From: Rohit Sharma Date: Mon, 7 Dec 2020 21:44:05 +0200 Subject: [PATCH 3/7] Create a private method to check if elem is shorter --- js/src/modal.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/js/src/modal.js b/js/src/modal.js index 1b8363c01..c06374d74 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -462,7 +462,7 @@ class Modal extends BaseComponent { // Adjust fixed content padding SelectorEngine.find(SELECTOR_FIXED_CONTENT) .forEach(element => { - if (window.innerWidth > element.clientWidth + this._scrollbarWidth) { + if (this._isShorterThanWindow(element)) { return } @@ -475,7 +475,7 @@ class Modal extends BaseComponent { // Adjust sticky content margin SelectorEngine.find(SELECTOR_STICKY_CONTENT) .forEach(element => { - if (window.innerWidth > element.clientWidth + this._scrollbarWidth) { + if (this._isShorterThanWindow(element)) { return } @@ -536,6 +536,10 @@ class Modal extends BaseComponent { return scrollbarWidth } + _isShorterThanWindow(element) { + return window.innerWidth > element.clientWidth + this._scrollbarWidth + } + // Static static jQueryInterface(config, relatedTarget) { From 5ddb40a6ecdaacb719a4e887a0d3dbac18afd81a Mon Sep 17 00:00:00 2001 From: Rohit Sharma Date: Tue, 8 Dec 2020 01:28:22 +0530 Subject: [PATCH 4/7] Update unit tests --- js/tests/unit/modal.spec.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/js/tests/unit/modal.spec.js b/js/tests/unit/modal.spec.js index 34e1ac2d8..ca76439eb 100644 --- a/js/tests/unit/modal.spec.js +++ b/js/tests/unit/modal.spec.js @@ -142,33 +142,28 @@ describe('Modal', () => { modal.toggle() }) - it('should not adjust the inline margin of sticky elements when element do not have full width', done => { + 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 margin should not be adjusted while opening') - modal.toggle() - }) - - modalEl.addEventListener('hidden.bs.modal', () => { - const currentMargin = Number.parseInt(window.getComputedStyle(stickyTopEl).marginRight, 10) - - expect(stickyTopEl.getAttribute('data-margin-right')).toEqual(null, 'data-margin-right should be cleared after closing') - expect(currentMargin).toEqual(originalMargin, 'sticky element margin should be reset after closing') + 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.toggle() + modal.show() }) it('should ignore values set via CSS when trying to restore body padding after closing', done => { From 807e528fea2d9322cd1d01d672e391b8b72b8a98 Mon Sep 17 00:00:00 2001 From: Rohit Sharma Date: Mon, 14 Dec 2020 22:12:01 +0530 Subject: [PATCH 5/7] WIP: Add methods to reuse code --- js/src/modal.js | 61 ++++++++++++++++++++----------------------------- 1 file changed, 25 insertions(+), 36 deletions(-) diff --git a/js/src/modal.js b/js/src/modal.js index c06374d74..d4927dd27 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -461,29 +461,11 @@ class Modal extends BaseComponent { // Adjust fixed content padding SelectorEngine.find(SELECTOR_FIXED_CONTENT) - .forEach(element => { - if (this._isShorterThanWindow(element)) { - return - } - - const actualPadding = element.style.paddingRight - const calculatedPadding = window.getComputedStyle(element)['padding-right'] - Manipulator.setDataAttribute(element, 'padding-right', actualPadding) - element.style.paddingRight = `${Number.parseFloat(calculatedPadding) + this._scrollbarWidth}px` - }) + .forEach(element => this._setElementAttributes(element, 'paddingRight')) // Adjust sticky content margin SelectorEngine.find(SELECTOR_STICKY_CONTENT) - .forEach(element => { - if (this._isShorterThanWindow(element)) { - return - } - - const actualMargin = element.style.marginRight - const calculatedMargin = window.getComputedStyle(element)['margin-right'] - Manipulator.setDataAttribute(element, 'margin-right', actualMargin) - element.style.marginRight = `${Number.parseFloat(calculatedMargin) - this._scrollbarWidth}px` - }) + .forEach(element => this._setElementAttributes(element, 'marginRight')) // Adjust body padding const actualPadding = document.body.style.paddingRight @@ -499,23 +481,11 @@ class Modal extends BaseComponent { _resetScrollbar() { // Restore fixed content padding SelectorEngine.find(SELECTOR_FIXED_CONTENT) - .forEach(element => { - const padding = Manipulator.getDataAttribute(element, 'padding-right') - if (typeof padding !== 'undefined') { - Manipulator.removeDataAttribute(element, 'padding-right') - element.style.paddingRight = padding - } - }) + .forEach(element => this._removeElementAttributes(element, 'paddingRight')) // Restore sticky content and navbar-toggler margin SelectorEngine.find(`${SELECTOR_STICKY_CONTENT}`) - .forEach(element => { - const margin = Manipulator.getDataAttribute(element, 'margin-right') - if (typeof margin !== 'undefined') { - Manipulator.removeDataAttribute(element, 'margin-right') - element.style.marginRight = margin - } - }) + .forEach(element => this._removeElementAttributes(element, 'marginRight')) // Restore body padding const padding = Manipulator.getDataAttribute(document.body, 'padding-right') @@ -536,8 +506,27 @@ class Modal extends BaseComponent { return scrollbarWidth } - _isShorterThanWindow(element) { - return window.innerWidth > element.clientWidth + this._scrollbarWidth + _setElementAttributes(element, cssProp) { + if (window.innerWidth > element.clientWidth + this._scrollbarWidth) { + return + } + + const actualValue = element.style[cssProp] + const computedValue = window.getComputedStyle(element)[cssProp] + Manipulator.setDataAttribute(element, cssProp, actualValue) + if (cssProp === 'marginRight') { + element.style[cssProp] = `${Number.parseFloat(computedValue) - this._scrollbarWidth}px` + } else { + element.style[cssProp] = `${Number.parseFloat(computedValue) + this._scrollbarWidth}px` + } + } + + _removeElementAttributes(element, cssProp) { + const cssValue = Manipulator.getDataAttribute(element, cssProp) + if (typeof cssValue !== 'undefined') { + Manipulator.removeDataAttribute(element, cssProp) + element.style[cssProp] = cssValue + } } // Static From 5ce207dd864b5e34302862ac93ccfa92a052bb51 Mon Sep 17 00:00:00 2001 From: XhmikosR Date: Tue, 15 Dec 2020 11:21:28 +0200 Subject: [PATCH 6/7] Update modal.js --- js/src/modal.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/src/modal.js b/js/src/modal.js index d4927dd27..610840335 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -513,7 +513,9 @@ class Modal extends BaseComponent { const actualValue = element.style[cssProp] const computedValue = window.getComputedStyle(element)[cssProp] + Manipulator.setDataAttribute(element, cssProp, actualValue) + if (cssProp === 'marginRight') { element.style[cssProp] = `${Number.parseFloat(computedValue) - this._scrollbarWidth}px` } else { From e5933330e1e3c363d240035d67f97ec094577d91 Mon Sep 17 00:00:00 2001 From: XhmikosR Date: Tue, 15 Dec 2020 11:23:25 +0200 Subject: [PATCH 7/7] Update modal.js --- js/src/modal.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/js/src/modal.js b/js/src/modal.js index 610840335..ea79420c8 100644 --- a/js/src/modal.js +++ b/js/src/modal.js @@ -516,11 +516,9 @@ class Modal extends BaseComponent { Manipulator.setDataAttribute(element, cssProp, actualValue) - if (cssProp === 'marginRight') { - element.style[cssProp] = `${Number.parseFloat(computedValue) - this._scrollbarWidth}px` - } else { - element.style[cssProp] = `${Number.parseFloat(computedValue) + this._scrollbarWidth}px` - } + element.style[cssProp] = cssProp === 'marginRight' ? + `${Number.parseFloat(computedValue) - this._scrollbarWidth}px` : + `${Number.parseFloat(computedValue) + this._scrollbarWidth}px` } _removeElementAttributes(element, cssProp) {