From 1f681eea6c2daaae05ff25a11a2855f0e0dc7303 Mon Sep 17 00:00:00 2001 From: Rick Zhou Date: Wed, 16 May 2018 12:53:25 -0700 Subject: [PATCH 1/5] modal resetScrollbar should revert the change only when the change was done in the first place, resolves #1831 --- src/components/modal/modal.js | 47 ++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js index bf8bd1d0cfe..18e1f84d2dd 100644 --- a/src/components/modal/modal.js +++ b/src/components/modal/modal.js @@ -752,6 +752,8 @@ export default { const computedStyle = window.getComputedStyle const body = document.body const scrollbarWidth = this.scrollbarWidth + this._marginChangedForScroll = [] + this._paddingChangedForScroll = [] // Adjust fixed content padding selectAll(Selector.FIXED_CONTENT).forEach(el => { const actualPadding = el.style.paddingRight @@ -759,6 +761,7 @@ export default { setAttr(el, 'data-padding-right', actualPadding) el.style.paddingRight = `${parseFloat(calculatedPadding) + scrollbarWidth}px` + this._paddingChangedForScroll.push(el) }) // Adjust sticky content margin selectAll(Selector.STICKY_CONTENT).forEach(el => { @@ -767,6 +770,7 @@ export default { setAttr(el, 'data-margin-right', actualMargin) el.style.marginRight = `${parseFloat(calculatedMargin) - scrollbarWidth}px` + this._marginChangedForScroll.push(el) }) // Adjust navbar-toggler margin selectAll(Selector.NAVBAR_TOGGLER).forEach(el => { @@ -775,6 +779,7 @@ export default { setAttr(el, 'data-margin-right', actualMargin) el.style.marginRight = `${parseFloat(calculatedMargin) + scrollbarWidth}px` + this._marginChangedForScroll.push(el) }) // Adjust body padding const actualPadding = body.style.paddingRight @@ -785,27 +790,29 @@ export default { } }, resetScrollbar () { - // Restore fixed content padding - selectAll(Selector.FIXED_CONTENT).forEach(el => { - if (hasAttr(el, 'data-padding-right')) { - el.style.paddingRight = getAttr(el, 'data-padding-right') || '' - removeAttr(el, 'data-padding-right') - } - }) - // Restore sticky content and navbar-toggler margin - selectAll( - `${Selector.STICKY_CONTENT}, ${Selector.NAVBAR_TOGGLER}` - ).forEach(el => { - if (hasAttr(el, 'data-margin-right')) { - el.style.marginRight = getAttr(el, 'data-margin-right') || '' - removeAttr(el, 'data-margin-right') + if (this._marginChangedForScroll && this._paddingChangedForScroll) { + // Restore fixed content padding + this._paddingChangedForScroll.forEach(el => { + if (hasAttr(el, 'data-padding-right')) { + el.style.paddingRight = getAttr(el, 'data-padding-right') || '' + removeAttr(el, 'data-padding-right') + } + }) + // Restore sticky content and navbar-toggler margin + this._marginChangedForScroll.forEach(el => { + if (hasAttr(el, 'data-margin-right')) { + el.style.marginRight = getAttr(el, 'data-margin-right') || '' + removeAttr(el, 'data-margin-right') + } + }) + this._paddingChangedForScroll = null + this._marginChangedForScroll = null + // Restore body padding + const body = document.body + if (hasAttr(body, 'data-padding-right')) { + body.style.paddingRight = getAttr(body, 'data-padding-right') || '' + removeAttr(body, 'data-padding-right') } - }) - // Restore body padding - const body = document.body - if (hasAttr(body, 'data-padding-right')) { - body.style.paddingRight = getAttr(body, 'data-padding-right') || '' - removeAttr(body, 'data-padding-right') } } }, From 7c9e9a720fe0573f7e39b0d5414910df927587fa Mon Sep 17 00:00:00 2001 From: Rick Zhou Date: Wed, 16 May 2018 13:48:57 -0700 Subject: [PATCH 2/5] add test to show modal --- src/components/modal/modal.spec.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index fe093268996..910946df16f 100755 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -17,4 +17,14 @@ describe('modal', async () => { await nextTick() expect(app.$refs.button).not.toHaveProperty('__BV_boundEventListeners__.click') }) + + it('Should show modal', async () => { + const { app: { $refs } } = window + const { modalButton, modal } = $refs + + modalButton.click() + + await nextTick() + expect(modal.$el.classList.contains('show')) + }) }) From ba5ef87782e148ddc3514d96097995fb7477624a Mon Sep 17 00:00:00 2001 From: Rick Zhou Date: Wed, 16 May 2018 14:25:40 -0700 Subject: [PATCH 3/5] add test to hide modal --- src/components/modal/modal.spec.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index 910946df16f..1e0b914b27e 100755 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -18,13 +18,19 @@ describe('modal', async () => { expect(app.$refs.button).not.toHaveProperty('__BV_boundEventListeners__.click') }) - it('Should show modal', async () => { + it('Should show hide modal', async () => { const { app: { $refs } } = window const { modalButton, modal } = $refs + // show the modal modalButton.click() - await nextTick() expect(modal.$el.classList.contains('show')) + + // hide the modal + let clickEvent = new MouseEvent('click', { bubbles: true, cancelable: true, view: window }) + modal.$el.dispatchEvent(clickEvent) + await nextTick() + expect(!modal.$el.classList.contains('show')) }) }) From 26a74f3235690a5703d3a726aa9bdf62498bba8c Mon Sep 17 00:00:00 2001 From: Rick Zhou Date: Wed, 16 May 2018 15:12:42 -0700 Subject: [PATCH 4/5] fix modal hide test --- src/components/modal/modal.spec.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index 1e0b914b27e..6da2e58d2d2 100755 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -25,12 +25,13 @@ describe('modal', async () => { // show the modal modalButton.click() await nextTick() - expect(modal.$el.classList.contains('show')) + expect(Array.isArray(modal._marginChangedForScroll)).toBe(true) // hide the modal - let clickEvent = new MouseEvent('click', { bubbles: true, cancelable: true, view: window }) - modal.$el.dispatchEvent(clickEvent) + modal.hide() await nextTick() - expect(!modal.$el.classList.contains('show')) + // manually run resetScrollbar because JSDOM doesn't support it + modal.resetScrollbar() + expect(modal._marginChangedForScroll).toBe(null) }) }) From 0131d8e740fe5e3592f074afc7423fbda5661991 Mon Sep 17 00:00:00 2001 From: Rick Zhou Date: Wed, 16 May 2018 16:13:23 -0700 Subject: [PATCH 5/5] add test for fixed and sticky content --- src/components/modal/fixtures/modal.html | 8 ++++++++ src/components/modal/modal.spec.js | 12 ++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/components/modal/fixtures/modal.html b/src/components/modal/fixtures/modal.html index f52c91bd0b5..26f8d12ae58 100755 --- a/src/components/modal/fixtures/modal.html +++ b/src/components/modal/fixtures/modal.html @@ -27,4 +27,12 @@ + Launch demo modal + + + + + + diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index 6da2e58d2d2..435ec0bd6e0 100755 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -20,18 +20,18 @@ describe('modal', async () => { it('Should show hide modal', async () => { const { app: { $refs } } = window - const { modalButton, modal } = $refs + const { modalButton2, modal2 } = $refs // show the modal - modalButton.click() + modalButton2.click() await nextTick() - expect(Array.isArray(modal._marginChangedForScroll)).toBe(true) + expect(Array.isArray(modal2._marginChangedForScroll)).toBe(true) // hide the modal - modal.hide() + modal2.hide() await nextTick() // manually run resetScrollbar because JSDOM doesn't support it - modal.resetScrollbar() - expect(modal._marginChangedForScroll).toBe(null) + modal2.resetScrollbar() + expect(modal2._marginChangedForScroll).toBe(null) }) })