From 54575418f7f6fbc967420da2c22359a4a03c669e Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Sat, 6 Apr 2019 22:19:00 -0300 Subject: [PATCH 1/7] fix(modal): prevent close on backdrop when click initiated inside modal content (fixes #3025) Fixes #3025 --- src/components/modal/modal.js | 39 ++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js index 41dfd58d249..b97b2f4592e 100644 --- a/src/components/modal/modal.js +++ b/src/components/modal/modal.js @@ -25,6 +25,9 @@ const OBSERVER_CONFIG = { attributeFilter: ['style', 'class'] } +// Options for DOM event listeners +const EVT_OPTIONS = { passive: true, capture: false } + export const props = { title: { type: String, @@ -221,8 +224,9 @@ export default Vue.extend({ is_transitioning: false, // Used for style control is_show: false, // Used for style control is_block: false, // Used for style control - is_opening: false, // Semaphore for preventing incorrect modal open counts - is_closing: false, // Semaphore for preventing incorrect modal open counts + is_opening: false, // To sginal that modal is in the process of opening + is_closing: false, // To signal that the modal is in the process of closing + ignoreBackdropClick: false, // Used to signify if click out listener should ignore the click isModalOverflowing: false, return_focus: this.returnFocus || null, // The following items are controlled by the modalManager instance @@ -517,12 +521,31 @@ export default Vue.extend({ this.emitOnRoot(`bv::modal::${type}`, bvEvt, bvEvt.modalId) }, // UI event handlers + onDialogMousedown(evt) { + // Watch to see if the matching mouseup event occurs outside the dialog + // And if it does, cancel the clickout handler + const modal = this.$refs.modal + const onceModalMouseup = evt => { + eventOff(modal, 'mouseup', onceModalMouseup, EVT_OPTIONS) + if (evt.target === modal) { + this.ignoreBackdropClick = true + } + } + eventOn(modal, 'mouseup', onceModalMouseup, EVT_OPTIONS) + }, onClickOut(evt) { // Do nothing if not visible, backdrop click disabled, or element // that generated click event is no longer in document if (!this.is_visible || this.noCloseOnBackdrop || !contains(document, evt.target)) { return } + /* istanbul ignore next: difficult to simulate in JSDOM */ + if (this.ignoreBackdropClick) { + // Click was initiated inside the modal content, but finished outside + // Set by the above onDialogMousedown handler + this.ignoreBackdropClick = false + return + } // If backdrop clicked, hide modal if (!contains(this.$refs.content, evt.target)) { this.hide('backdrop') @@ -552,15 +575,14 @@ export default Vue.extend({ // Turn on/off focusin listener setEnforceFocus(on) { const method = on ? eventOn : eventOff - method(document, 'focusin', this.focusHandler, { passive: true, capture: false }) + method(document, 'focusin', this.focusHandler, EVT_OPTIONS) }, // Resize listener setResizeEvent(on) { - const options = { passive: true, capture: false } const method = on ? eventOn : eventOff // These events should probably also check if body is overflowing - method(window, 'resize', this.checkModalOverflow, options) - method(window, 'orientationchange', this.checkModalOverflow, options) + method(window, 'resize', this.checkModalOverflow, EVT_OPTIONS) + method(window, 'orientationchange', this.checkModalOverflow, EVT_OPTIONS) }, // Root listener handlers showHandler(id, triggerEl) { @@ -758,7 +780,10 @@ export default Vue.extend({ 'div', { staticClass: 'modal-dialog', - class: this.dialogClasses + class: this.dialogClasses, + on: { + mousedown: this.onDialogMousedown + } }, [modalContent] ) From 69b64c9a9c0ca360882c8b1c182d03ec9c3e1d9a Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Sat, 6 Apr 2019 22:21:22 -0300 Subject: [PATCH 2/7] Update modal.js --- src/components/modal/modal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js index b97b2f4592e..c5f1c0847f3 100644 --- a/src/components/modal/modal.js +++ b/src/components/modal/modal.js @@ -521,7 +521,7 @@ export default Vue.extend({ this.emitOnRoot(`bv::modal::${type}`, bvEvt, bvEvt.modalId) }, // UI event handlers - onDialogMousedown(evt) { + onDialogMousedown(evt) /* istanbul ignore next: difficult to test in JSDOM */ { // Watch to see if the matching mouseup event occurs outside the dialog // And if it does, cancel the clickout handler const modal = this.$refs.modal From 2e1b5cc7c3de6420e11ea4cd22a0664ebb9530d2 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Sat, 6 Apr 2019 22:37:16 -0300 Subject: [PATCH 3/7] Update modal.spec.js --- src/components/modal/modal.spec.js | 79 ++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index edd3ffd1dd0..d5d40ceba3b 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -581,6 +581,85 @@ describe('modal', () => { wrapper.destroy() }) + + it('mousedown inside followed by mouse up outside (click) does not close modal', async () => { + let trigger = null + let called = false + const wrapper = mount(BModal, { + attachToDocument: true, + stubs: { + transition: false + }, + propsData: { + id: 'test', + visible: true + }, + listeners: { + hide: bvEvent => { + called = true + trigger = bvEvent.trigger + } + } + }) + + expect(wrapper.isVueInstance()).toBe(true) + + await wrapper.vm.$nextTick() + await waitAF() + await wrapper.vm.$nextTick() + await waitAF() + + const $modal = wrapper.find('div.modal') + expect($modal.exists()).toBe(true) + + const $dialog = wrapper.find('div.modal-dialog') + expect($dialog.exists()).toBe(true) + + const $footer = wrapper.find('div.modal-footer') + expect($dialog.exists()).toBe(true) + + expect($modal.element.style.display).toEqual('') + + expect(wrapper.emitted('hide')).not.toBeDefined() + expect(trigger).toEqual(null) + + // Try and close modal via a "dragged" click out + // starting from inside modal and finishing on backdrop + $dialog.trigger('mousedown') + $modal.trigger('mouseup') + $modal.trigger('click') + + await wrapper.vm.$nextTick() + await waitAF() + await wrapper.vm.$nextTick() + await waitAF() + + expect(called).toEqual(false) + expect(trigger).toEqual(null) + + // Modal should not be closed + expect($modal.element.style.display).toEqual('') + + // Try and close modal via a "dragged" click out + // starting from inside modal and finishing on backdrop + $footer.trigger('mousedown') + $modal.trigger('mouseup') + $modal.trigger('click') + + await wrapper.vm.$nextTick() + await waitAF() + await wrapper.vm.$nextTick() + await waitAF() + + expect(called).toEqual(false) + expect(trigger).toEqual(null) + + // Modal should not be closed + expect($modal.element.style.display).toEqual('') + + wrapper.destroy() + }) + it('$root bv::show::modal and bv::hide::modal work', async () => { const wrapper = mount(BModal, { attachToDocument: true, From 46751f3d5879f07b5651bcf6254d54dc62be3a55 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Sat, 6 Apr 2019 22:38:01 -0300 Subject: [PATCH 4/7] Update modal.js --- src/components/modal/modal.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js index c5f1c0847f3..4038e03f2d3 100644 --- a/src/components/modal/modal.js +++ b/src/components/modal/modal.js @@ -521,7 +521,7 @@ export default Vue.extend({ this.emitOnRoot(`bv::modal::${type}`, bvEvt, bvEvt.modalId) }, // UI event handlers - onDialogMousedown(evt) /* istanbul ignore next: difficult to test in JSDOM */ { + onDialogMousedown(evt) { // Watch to see if the matching mouseup event occurs outside the dialog // And if it does, cancel the clickout handler const modal = this.$refs.modal @@ -539,7 +539,6 @@ export default Vue.extend({ if (!this.is_visible || this.noCloseOnBackdrop || !contains(document, evt.target)) { return } - /* istanbul ignore next: difficult to simulate in JSDOM */ if (this.ignoreBackdropClick) { // Click was initiated inside the modal content, but finished outside // Set by the above onDialogMousedown handler From 875508dc2175c044d49a2b4ec795ee13242f8b03 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Sat, 6 Apr 2019 22:39:40 -0300 Subject: [PATCH 5/7] lint --- src/components/modal/modal.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index d5d40ceba3b..586056ca6b0 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -581,7 +581,6 @@ describe('modal', () => { wrapper.destroy() }) - it('mousedown inside followed by mouse up outside (click) does not close modal', async () => { let trigger = null let called = false From f32d067998d44c2b3455f1e54274e6cb995aa301 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Sat, 6 Apr 2019 22:43:19 -0300 Subject: [PATCH 6/7] Update modal.spec.js --- src/components/modal/modal.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index 586056ca6b0..946738b9241 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -614,8 +614,8 @@ describe('modal', () => { const $dialog = wrapper.find('div.modal-dialog') expect($dialog.exists()).toBe(true) - const $footer = wrapper.find('div.modal-footer') - expect($dialog.exists()).toBe(true) + const $footer = wrapper.find('footer.modal-footer') + expect($footer.exists()).toBe(true) expect($modal.element.style.display).toEqual('') From 282b19af81b4f298cebda26da201baf5cf8a5224 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Sat, 6 Apr 2019 23:18:25 -0300 Subject: [PATCH 7/7] Update modal.spec.js --- src/components/modal/modal.spec.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index 946738b9241..c2c23ad1444 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -656,6 +656,20 @@ describe('modal', () => { // Modal should not be closed expect($modal.element.style.display).toEqual('') + // Try and close modal via click out + $modal.trigger('click') + + await wrapper.vm.$nextTick() + await waitAF() + await wrapper.vm.$nextTick() + await waitAF() + + expect(called).toEqual(true) + expect(trigger).toEqual('backdrop') + + // Modal should now be closed + expect($modal.element.style.display).toEqual('none') + wrapper.destroy() })