Skip to content

fix(modal): prevent close on backdrop when click initiated inside modal content (fixes #3025) #3029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions src/components/modal/modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -517,12 +521,30 @@ 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
}
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')
Expand Down Expand Up @@ -552,15 +574,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) {
Expand Down Expand Up @@ -758,7 +779,10 @@ export default Vue.extend({
'div',
{
staticClass: 'modal-dialog',
class: this.dialogClasses
class: this.dialogClasses,
on: {
mousedown: this.onDialogMousedown
}
},
[modalContent]
)
Expand Down
92 changes: 92 additions & 0 deletions src/components/modal/modal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,98 @@ 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('footer.modal-footer')
expect($footer.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('')

// 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()
})

it('$root bv::show::modal and bv::hide::modal work', async () => {
const wrapper = mount(BModal, {
attachToDocument: true,
Expand Down