From 424e5feec3cbfcd618658d60b138816fb95a0bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Fri, 5 Apr 2019 10:48:35 +0200 Subject: [PATCH 01/11] fix(modal): make `zindexOffset` configurable in config and lower default --- docs/plugins/bootstrap-vue.js | 5 +- src/components/modal/modal.js | 97 +++++++++++++++++++---------------- src/utils/config.js | 2 + 3 files changed, 58 insertions(+), 46 deletions(-) diff --git a/docs/plugins/bootstrap-vue.js b/docs/plugins/bootstrap-vue.js index 4b3ae0e1349..ded2ebf784d 100644 --- a/docs/plugins/bootstrap-vue.js +++ b/docs/plugins/bootstrap-vue.js @@ -1,4 +1,7 @@ import Vue from 'vue' import BootstrapVue from '../../src' -Vue.use(BootstrapVue) +Vue.use(BootstrapVue, { + // Make sure modals in the docs are above anything else + BModal: { zIndexOffset: 2000 } +}) diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js index a8d740526fa..4194212880a 100644 --- a/src/components/modal/modal.js +++ b/src/components/modal/modal.js @@ -44,8 +44,8 @@ const OBSERVER_CONFIG = { attributeFilter: ['style', 'class'] } -// modal wrapper ZINDEX offset incrememnt -const ZINDEX_OFFSET = 2000 +// Modal wrapper ZINDEX offset +const ZINDEX_OFFSET = getComponentConfig(NAME, 'zIndexOffset') // Modal open count helpers function getModalOpenCount() { @@ -67,17 +67,17 @@ function decrementModalOpenCount() { // Returns the current visible modal highest z-index function getModalMaxZIndex() { - return selectAll('div.modal') /* find all modals that are in document */ - .filter(isVisible) /* filter only visible ones */ - .map(m => m.parentElement) /* select the outer div */ - .reduce((max, el) => { - /* compute the highest z-index */ - return Math.max(max, parseInt(el.style.zIndex || 0, 10)) - }, 0) + return ( + selectAll('div.modal') // Find all modals that are in document + .filter(isVisible) // Filter only visible ones + .map(m => m.parentElement) // Select the outer div + // Compute the highest z-index + .reduce((max, el) => Math.max(max, parseInt(el.style.zIndex || 0, 10)), 0) + ) } -// Returns the next z-index to be used by a modal to ensure proper stacking -// regardless of document order. Increments by 2000 +// Returns the next z-index to be used by a modal to ensure proper +// stacking regardless of document order function getModalNextZIndex() { return getModalMaxZIndex() + ZINDEX_OFFSET } @@ -259,7 +259,10 @@ export default { }, okVariant: { type: String, - default: () => getComponentConfig(NAME, 'okVariant') + default: () => { + console.log(getComponentConfig(NAME, 'zIndexOffset')) + return getComponentConfig(NAME, 'okVariant') + } }, lazy: { type: Boolean, @@ -272,13 +275,13 @@ export default { }, data() { return { - is_hidden: this.lazy || false, // for lazy modals - is_visible: false, // controls modal visible state + is_hidden: this.lazy || false, // For lazy modals + is_visible: false, // Controls modal visible state 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 previnting incorrect modal open counts - is_closing: false, // Semapbore for preventing incorrect modal open counts + is_opening: false, // Semaphore for preventing incorrect modal open counts + is_closing: false, // Semaphore for preventing incorrect modal open counts scrollbarWidth: 0, zIndex: ZINDEX_OFFSET, // z-index for modal stacking isTop: true, // If the modal is the topmost opened modal @@ -347,7 +350,7 @@ export default { }, modalOuterStyle() { return { - // We only set these styles on the stacked modals (ones with next z-index > 0). + // We only set these styles on the stacked modals (z-index > 0) position: 'absolute', zIndex: this.zIndex } @@ -366,13 +369,14 @@ export default { }, mounted() { // Listen for events from others to either open or close ourselves - // And listen to all modals to enable/disable enforce focus + // and listen to all modals to enable/disable enforce focus this.listenOnRoot('bv::show::modal', this.showHandler) this.listenOnRoot('bv::modal::shown', this.shownHandler) this.listenOnRoot('bv::hide::modal', this.hideHandler) this.listenOnRoot('bv::modal::hidden', this.hiddenHandler) this.listenOnRoot('bv::toggle::modal', this.toggleHandler) - // Listen for bv:modal::show events, and close ourselves if the opening modal not us + // Listen for `bv:modal::show events`, and close ourselves if the + // opening modal not us this.listenOnRoot('bv::modal::show', this.modalListener) // Initially show modal? if (this.visible === true) { @@ -385,7 +389,7 @@ export default { this._observer.disconnect() this._observer = null } - // Ensure our root "once" listener is gone + // Ensure our root 'once' listener is gone this.$root.$off('bv::modal::hidden', this.doShow) this.setEnforceFocus(false) this.setResizeEvent(false) @@ -406,12 +410,12 @@ export default { // Public Methods show() { if (this.is_visible || this.is_opening) { - // if already open, on in the process of opening, do nothing + // If already open, on in the process of opening, do nothing /* istanbul ignore next */ return } if (this.is_closing) { - // if we are in the process of closing, wait until hidden before re-opening + // If we are in the process of closing, wait until hidden before re-opening /* istanbul ignore next: very difficult to test */ this.$once('hidden', this.show) /* istanbul ignore next */ @@ -453,7 +457,7 @@ export default { vueTarget: this, target: this.$refs.modal, modalId: this.safeId(), - // this could be the trigger element/component reference + // This could be the trigger element/component reference relatedTarget: null, isOK: trigger || null, trigger: trigger || null, @@ -474,7 +478,7 @@ export default { this.is_closing = false return } - // stop observing for content changes + // Stop observing for content changes if (this._observer) { this._observer.disconnect() this._observer = null @@ -498,7 +502,8 @@ export default { // Place modal in DOM if lazy this.is_hidden = false this.$nextTick(() => { - // We do this in nextTick to ensure the modal is in DOM first before we show it + // We do this in `$nextTick()` to ensure the modal is in DOM first + // before we show it this.is_visible = true this.is_opening = false this.$emit('change', true) @@ -510,7 +515,7 @@ export default { ) }) }, - // Transition Handlers + // Transition handlers onBeforeEnter() { this.getScrollbarWidth() this.is_transitioning = true @@ -581,10 +586,10 @@ export default { this.$emit(type, bvEvt) this.$root.$emit(`bv::modal::${type}`, bvEvt, this.safeId()) }, - // UI Event Handlers + // UI event handlers onClickOut(evt) { - // Do nothing if not visible, backdrop click disabled, or element that generated - // click event is no longer in document + // 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 } @@ -619,14 +624,14 @@ export default { const method = on ? eventOn : eventOff method(document, 'focusin', this.focusHandler, { passive: true, capture: false }) }, - // Resize Listener + // Resize listener setResizeEvent(on) { const options = { passive: true, capture: false } const method = on ? eventOn : eventOff method(window, 'resize', this.adjustDialog, options) method(window, 'orientationchange', this.adjustDialog, options) }, - // Root Listener handlers + // Root listener handlers showHandler(id, triggerEl) { if (id === this.id) { this.return_focus = triggerEl || null @@ -669,12 +674,12 @@ export default { const modal = this.$refs.modal const activeElement = document.activeElement if (activeElement && contains(modal, activeElement)) { - // If activeElement is child of modal or is modal, no need to change focus + // If `activeElement` is child of modal or is modal, no need to change focus return } if (modal) { - // make sure top of modal is showing (if longer than the viewport) and - // focus the modal content wrapper + // Make sure top of modal is showing (if longer than the viewport) + // and focus the modal content wrapper this.$nextTick(() => { modal.scrollTop = 0 modal.focus() @@ -682,7 +687,7 @@ export default { } }, returnFocusTo() { - // Prefer returnFocus prop over event specified return_focus value + // Prefer `returnFocus` prop over event specified `return_focus` value let el = this.returnFocus || this.return_focus || null if (typeof el === 'string') { // CSS Selector @@ -740,14 +745,15 @@ export default { setScrollbar() { const body = document.body // Storage place to cache changes to margins and padding - // Note: THis assumes the following element types are not added to the - // document after hte modal has opened. + // Note: This assumes the following element types are not added + // to the document after the modal has opened. body._paddingChangedForModal = body._paddingChangedForModal || [] body._marginChangedForModal = body._marginChangedForModal || [] /* istanbul ignore if: get Computed Style can't be tested in JSDOM */ if (this.isBodyOverflowing) { - // Note: DOMNode.style.paddingRight returns the actual value or '' if not set - // while $(DOMNode).css('padding-right') returns the calculated value or 0 if not set + // Note: `DOMNode.style.paddingRight returns` the actual value or '' + // if not set while `$(DOMNode).css('padding-right')` returns the + // calculated value or 0 if not set const scrollbarWidth = this.scrollbarWidth // Adjust fixed content padding selectAll(Selector.FIXED_CONTENT).forEach(el => { @@ -811,7 +817,7 @@ export default { }, render(h) { const $slots = this.$slots - // Modal Header + // Modal header let header = h(false) if (!this.hideHeader) { let modalHeader = $slots['modal-header'] @@ -853,7 +859,7 @@ export default { [modalHeader] ) } - // Modal Body + // Modal body const body = h( 'div', { @@ -917,7 +923,7 @@ export default { [modalFooter] ) } - // Assemble Modal Content + // Assemble modal content const modalContent = h( 'div', { @@ -932,7 +938,7 @@ export default { }, [header, body, footer] ) - // Modal Dialog wrapper + // Modal dialog wrapper const modalDialog = h( 'div', { @@ -1003,7 +1009,8 @@ export default { [$slots['modal-backdrop']] ) } - // Tab trap to prevent page from scrolling to next element in tab index during enforce focus tab cycle + // Tab trap to prevent page from scrolling to next element in tab index + // during enforce focus tab cycle let tabTrap = h(false) if (this.is_visible && this.isTop && !this.noEnforceFocus) { tabTrap = h('div', { attrs: { tabindex: '0' } }) @@ -1021,7 +1028,7 @@ export default { [modal, tabTrap, backdrop] ) } - // Wrap in DIV to maintain thi.$el reference for hide/show method aceess + // Wrap in DIV to maintain `this.$el` reference for hide/show method access return h('div', {}, [outer]) } } diff --git a/src/utils/config.js b/src/utils/config.js index 47493a509b5..6660af5575a 100644 --- a/src/utils/config.js +++ b/src/utils/config.js @@ -76,6 +76,8 @@ const DEFAULTS = { blankColor: 'transparent' }, BModal: { + // Defaults to the Boostrap default for the modal backdrop + zIndexOffset: 1040, cancelTitle: 'Cancel', cancelVariant: 'secondary', okTitle: 'OK', From 31796fdae735d4606ca8118f67bfcbefca30693a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Fri, 5 Apr 2019 10:57:39 +0200 Subject: [PATCH 02/11] Update modal.spec.js --- src/components/modal/modal.spec.js | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index 7ea43dc6bf8..061d1e83d46 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -16,21 +16,21 @@ describe('modal', () => { await wrapper.vm.$nextTick() // This outer DIV will go away once we migrate to Portal-Vue - // As all modals will be lazy + // as all modals will be lazy expect(wrapper.is('div')).toBe(true) expect(wrapper.classes().length).toBe(0) - // Main outer wraper (has z-index, etc)... the stacker div + // Main outer wrapper (has z-index, etc)... the stacker div const $outer = createWrapper(wrapper.element.firstElementChild) expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual('2000') + expect($outer.element.style.zIndex).toEqual('1040') // Should not have a backdrop expect($outer.find('div.modal-backdrop').exists()).toBe(false) - // Main modal wraper + // Main modal wrapper const $modal = $outer.find('div.modal') expect($modal.exists()).toBe(true) expect($modal.attributes('id')).toBeDefined() @@ -92,18 +92,18 @@ describe('modal', () => { await waitAF() // This outer DIV will go away once we migrate to Portal-Vue - // As all modals will be lazy + // as all modals will be lazy expect(wrapper.is('div')).toBe(true) expect(wrapper.classes().length).toBe(0) - // Main outer wraper (has z-index, etc)... the stacker div + // Main outer wrapper (has z-index, etc)... the stacker div const $outer = createWrapper(wrapper.element.firstElementChild) expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') expect($outer.element.style.zIndex).toEqual('2000') - // Main modal wraper + // Main modal wrapper const $modal = $outer.find('div.modal') expect($modal.exists()).toBe(true) expect($modal.attributes('id')).toBeDefined() @@ -139,7 +139,7 @@ describe('modal', () => { attachToDocument: true, stubs: { // Disable the use of transitionStub fake transition - // AS it doesn't run transition hooks + // as it doesn't run transition hooks transition: false }, propsData: { @@ -165,18 +165,18 @@ describe('modal', () => { // expect(body.getAttribute('data-modal-open-count')).toEqual('1') // This outer DIV will go away once we migrate to Portal-Vue - // As all modals will be lazy + // as all modals will be lazy expect(wrapper.is('div')).toBe(true) expect(wrapper.classes().length).toBe(0) - // Main outer wraper (has z-index, etc)... the stacker div + // Main outer wrapper (has z-index, etc)... the stacker div const $outer = createWrapper(wrapper.element.firstElementChild) expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') expect($outer.element.style.zIndex).toEqual('2000') - // Main modal wraper + // Main modal wrapper const $modal = $outer.find('div.modal') expect($modal.exists()).toBe(true) expect($modal.attributes('aria-hidden')).not.toBeDefined() @@ -225,9 +225,7 @@ describe('modal', () => { }) describe('default button content, classes and attributes', () => { - // - // We may want to move these tests into individual files for managability - // + // We may want to move these tests into individual files for manageability it('default footer ok and cancel buttons', async () => { const wrapper = mount(Modal) expect(wrapper).toBeDefined() @@ -568,7 +566,7 @@ describe('modal', () => { expect($modal.element.style.display).toEqual('none') - // Try and open modal via bv::show::modal + // Try and open modal via `bv::show::modal` wrapper.vm.$root.$emit('bv::show::modal', 'test') await wrapper.vm.$nextTick() @@ -579,7 +577,7 @@ describe('modal', () => { // Modal should now be open expect($modal.element.style.display).toEqual('') - // Try and close modal via bv::hide::modal + // Try and close modal via `bv::hide::modal` wrapper.vm.$root.$emit('bv::hide::modal', 'test') await wrapper.vm.$nextTick() From 806615acf251041d31dcec1170f4b7ebb47e0764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Fri, 5 Apr 2019 11:00:05 +0200 Subject: [PATCH 03/11] 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 061d1e83d46..e37528578f6 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -101,7 +101,7 @@ describe('modal', () => { expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual('2000') + expect($outer.element.style.zIndex).toEqual('1040') // Main modal wrapper const $modal = $outer.find('div.modal') @@ -174,7 +174,7 @@ describe('modal', () => { expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual('2000') + expect($outer.element.style.zIndex).toEqual('1040') // Main modal wrapper const $modal = $outer.find('div.modal') From a7fb0b3d2f1f1d3455d523ef0d31007a5b585e1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacob=20M=C3=BCller?= Date: Fri, 5 Apr 2019 11:29:15 +0200 Subject: [PATCH 04/11] Update modal.js --- src/components/modal/modal.js | 39 +++++++++++++---------------------- 1 file changed, 14 insertions(+), 25 deletions(-) diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js index 4194212880a..811bfec0506 100644 --- a/src/components/modal/modal.js +++ b/src/components/modal/modal.js @@ -44,29 +44,19 @@ const OBSERVER_CONFIG = { attributeFilter: ['style', 'class'] } -// Modal wrapper ZINDEX offset -const ZINDEX_OFFSET = getComponentConfig(NAME, 'zIndexOffset') - // Modal open count helpers -function getModalOpenCount() { - return parseInt(getAttr(document.body, 'data-modal-open-count') || 0, 10) -} +const getModalOpenCount = () => parseInt(getAttr(document.body, 'data-modal-open-count') || 0, 10) -function setModalOpenCount(count) { +const setModalOpenCount = count => { setAttr(document.body, 'data-modal-open-count', String(count)) return count } -function incrementModalOpenCount() { - return setModalOpenCount(getModalOpenCount() + 1) -} - -function decrementModalOpenCount() { - return setModalOpenCount(Math.max(getModalOpenCount() - 1, 0)) -} +const incrementModalOpenCount = () => setModalOpenCount(getModalOpenCount() + 1) +const decrementModalOpenCount = () => setModalOpenCount(Math.max(getModalOpenCount() - 1, 0)) // Returns the current visible modal highest z-index -function getModalMaxZIndex() { +const getModalMaxZIndex = () => { return ( selectAll('div.modal') // Find all modals that are in document .filter(isVisible) // Filter only visible ones @@ -76,11 +66,11 @@ function getModalMaxZIndex() { ) } +const getModalZIndexOffset = () => getComponentConfig(NAME, 'zIndexOffset') + // Returns the next z-index to be used by a modal to ensure proper // stacking regardless of document order -function getModalNextZIndex() { - return getModalMaxZIndex() + ZINDEX_OFFSET -} +const getModalNextZIndex = () => getModalMaxZIndex() + getModalZIndexOffset() // @vue/component export default { @@ -259,10 +249,7 @@ export default { }, okVariant: { type: String, - default: () => { - console.log(getComponentConfig(NAME, 'zIndexOffset')) - return getComponentConfig(NAME, 'okVariant') - } + default: () => getComponentConfig(NAME, 'okVariant') }, lazy: { type: Boolean, @@ -283,7 +270,7 @@ export default { is_opening: false, // Semaphore for preventing incorrect modal open counts is_closing: false, // Semaphore for preventing incorrect modal open counts scrollbarWidth: 0, - zIndex: ZINDEX_OFFSET, // z-index for modal stacking + zIndex: 0, // z-index for modal stacking isTop: true, // If the modal is the topmost opened modal isBodyOverflowing: false, return_focus: this.returnFocus || null @@ -364,8 +351,10 @@ export default { } }, created() { - // create non-reactive property + // Define non-reactive properties this._observer = null + // Set initial z-index defined in the config + this.zIndex = getModalZIndexOffset() }, mounted() { // Listen for events from others to either open or close ourselves @@ -567,7 +556,7 @@ export default { this.setEnforceFocus(false) this.$nextTick(() => { this.is_hidden = this.lazy || false - this.zIndex = ZINDEX_OFFSET + this.zIndex = getModalZIndexOffset() this.returnFocusTo() this.is_closing = false const hiddenEvt = new BvEvent('hidden', { From 9f7702e039cb93d1d92c323451e55c9c3111a0e3 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Fri, 5 Apr 2019 13:05:14 -0300 Subject: [PATCH 05/11] add a few istanbul ignore statements for branching that can't be tested in JSDOM --- src/components/modal/modal.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js index 811bfec0506..af7268ffa28 100644 --- a/src/components/modal/modal.js +++ b/src/components/modal/modal.js @@ -708,11 +708,13 @@ export default { } const modal = this.$refs.modal const isModalOverflowing = modal.scrollHeight > document.documentElement.clientHeight + /* istanbul ignore if: can't test in JSDOM */ if (!this.isBodyOverflowing && isModalOverflowing) { modal.style.paddingLeft = `${this.scrollbarWidth}px` } else { modal.style.paddingLeft = '' } + /* istanbul ignore else: can't test in JSDOM */ if (this.isBodyOverflowing && !isModalOverflowing) { modal.style.paddingRight = `${this.scrollbarWidth}px` } else { @@ -780,6 +782,7 @@ export default { if (body._paddingChangedForModal) { // Restore fixed content padding body._paddingChangedForModal.forEach(el => { + /* istanbul ignore next: can't test in JSDOM */ if (hasAttr(el, 'data-padding-right')) { el.style.paddingRight = getAttr(el, 'data-padding-right') || '' removeAttr(el, 'data-padding-right') @@ -789,6 +792,7 @@ export default { if (body._marginChangedForModal) { // Restore sticky content and navbar-toggler margin body._marginChangedForModal.forEach(el => { + /* istanbul ignore next: can't test in JSDOM */ if (hasAttr(el, 'data-margin-right')) { el.style.marginRight = getAttr(el, 'data-margin-right') || '' removeAttr(el, 'data-margin-right') From ee68429bb30659dcd6bc1e59cdbdccd9e0508917 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Fri, 5 Apr 2019 13:21:23 -0300 Subject: [PATCH 06/11] Update modal.spec.js --- src/components/modal/modal.spec.js | 32 ++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index e37528578f6..ca552057450 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -4,9 +4,32 @@ import { mount, createWrapper } from '@vue/test-utils' const waitAF = () => new Promise(resolve => requestAnimationFrame(resolve)) describe('modal', () => { + const origGetBCR = Element.prototype.getBoundingClientRect + + beforeEach(() => { + // Mock getBCR so that the isVisible(el) test returns true + // Needed for z-index checks + Element.prototype.getBoundingClientRect = jest.fn(() => { + return { + width: 24, + height: 24, + top: 0, + left: 0, + bottom: 0, + right: 0 + } + }) + }) + + afterEach(() => { + // Restore prototype + Element.prototype.getBoundingClientRect = origGetBCR + }) + describe('structure', () => { it('has expected default structure', async () => { const wrapper = mount(Modal, { + attachToDocument: true, propsData: { id: 'test' } @@ -56,6 +79,7 @@ describe('modal', () => { it('has expected structure when lazy', async () => { const wrapper = mount(Modal, { + attachToDocument: true, propsData: { lazy: true } @@ -227,7 +251,9 @@ describe('modal', () => { describe('default button content, classes and attributes', () => { // We may want to move these tests into individual files for manageability it('default footer ok and cancel buttons', async () => { - const wrapper = mount(Modal) + const wrapper = mount(Modal, { + attachToDocument: true, + }) expect(wrapper).toBeDefined() const $buttons = wrapper.findAll('footer button') @@ -251,7 +277,9 @@ describe('modal', () => { }) it('default header close button', async () => { - const wrapper = mount(Modal) + const wrapper = mount(Modal, { + attachToDocument: true, + }) expect(wrapper).toBeDefined() const $buttons = wrapper.findAll('header button') From d8703835dc5dc2574f8e0b766e40e4f4627a5113 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Fri, 5 Apr 2019 13:37:26 -0300 Subject: [PATCH 07/11] Update modal.spec.js --- src/components/modal/modal.spec.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index ca552057450..501fb8a05f7 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -1,5 +1,9 @@ import Modal from './modal' import { mount, createWrapper } from '@vue/test-utils' +import { getComponentConfig } from `../../utils/config' + +// Grab the configured base index +const CONFIG_ZINDEX = getComponentConfig('BModal', 'zIndexOffset') const waitAF = () => new Promise(resolve => requestAnimationFrame(resolve)) @@ -35,6 +39,8 @@ describe('modal', () => { } }) + expect(CONFIG_ZINDEX).toBeGreaterThan(0) + expect(wrapper.isVueInstance()).toBe(true) await wrapper.vm.$nextTick() @@ -48,7 +54,7 @@ describe('modal', () => { expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual('1040') + expect($outer.element.style.zIndex).toEqual(`${CONFIG_ZINDEX}`) // Should not have a backdrop expect($outer.find('div.modal-backdrop').exists()).toBe(false) @@ -125,7 +131,7 @@ describe('modal', () => { expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual('1040') + expect($outer.element.style.zIndex).toEqual(`${CONFIG_ZINDEX}`) // Main modal wrapper const $modal = $outer.find('div.modal') @@ -198,7 +204,7 @@ describe('modal', () => { expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual('1040') + expect($outer.element.style.zIndex).toEqual(`${CONFIG_ZINDEX}`) // Main modal wrapper const $modal = $outer.find('div.modal') @@ -252,7 +258,7 @@ describe('modal', () => { // We may want to move these tests into individual files for manageability it('default footer ok and cancel buttons', async () => { const wrapper = mount(Modal, { - attachToDocument: true, + attachToDocument: true }) expect(wrapper).toBeDefined() @@ -278,7 +284,7 @@ describe('modal', () => { it('default header close button', async () => { const wrapper = mount(Modal, { - attachToDocument: true, + attachToDocument: true }) expect(wrapper).toBeDefined() From 7fea63f181a752841d4f53c76e6d3e14bcc2afba Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Fri, 5 Apr 2019 13:42:31 -0300 Subject: [PATCH 08/11] Update dom.js --- src/utils/dom.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/utils/dom.js b/src/utils/dom.js index 500b1160844..e063b2d4697 100644 --- a/src/utils/dom.js +++ b/src/utils/dom.js @@ -60,6 +60,10 @@ export const isVisible = el => /* istanbul ignore next: getBoundingClientRect() if (!isElement(el) || !contains(document.body, el)) { return false } + if (el.style.display === 'none') { + // We do this check to help with vue-test-utils when using v-show + return false + } // All browsers support getBoundingClientRect(), except JSDOM as it returns all 0's for values :( // So any tests that need isVisible will fail in JSDOM const bcr = getBCR(el) From 709e25062c5a1343619abcd3d9a661541420a68d Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Fri, 5 Apr 2019 13:44:22 -0300 Subject: [PATCH 09/11] lint --- src/components/modal/modal.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index 501fb8a05f7..8e632d559e4 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -1,6 +1,6 @@ import Modal from './modal' import { mount, createWrapper } from '@vue/test-utils' -import { getComponentConfig } from `../../utils/config' +import { getComponentConfig } from '../../utils/config' // Grab the configured base index const CONFIG_ZINDEX = getComponentConfig('BModal', 'zIndexOffset') From eea82a2472aab0ad0c7218093ca81c69c4bc6cfa Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Fri, 5 Apr 2019 13:49:24 -0300 Subject: [PATCH 10/11] Update dom.js --- src/utils/dom.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/utils/dom.js b/src/utils/dom.js index e063b2d4697..dd9ec1ea36f 100644 --- a/src/utils/dom.js +++ b/src/utils/dom.js @@ -56,7 +56,7 @@ export const isElement = el => { } // Determine if an HTML element is visible - Faster than CSS check -export const isVisible = el => /* istanbul ignore next: getBoundingClientRect() doesn't work in JSDOM */ { +export const isVisible = el => { if (!isElement(el) || !contains(document.body, el)) { return false } @@ -66,6 +66,7 @@ export const isVisible = el => /* istanbul ignore next: getBoundingClientRect() } // All browsers support getBoundingClientRect(), except JSDOM as it returns all 0's for values :( // So any tests that need isVisible will fail in JSDOM + // Except when we override the getBCR prototype in some tests const bcr = getBCR(el) return Boolean(bcr && bcr.height > 0 && bcr.width > 0) } From 00a9fbb389c40fcd39badba7809e2609757ef839 Mon Sep 17 00:00:00 2001 From: Troy Morehouse Date: Sat, 6 Apr 2019 21:05:00 -0300 Subject: [PATCH 11/11] z-index handling adjustments --- docs/plugins/bootstrap-vue.js | 5 +- src/components/modal/helpers/modal-manager.js | 221 ++++++++++++ src/components/modal/modal.js | 333 ++++-------------- src/components/modal/modal.spec.js | 89 ++++- src/mixins/listen-on-root.js | 24 ++ src/mixins/listen-on-root.spec.js | 73 ++++ src/utils/config.js | 2 - src/utils/dom.js | 1 + 8 files changed, 478 insertions(+), 270 deletions(-) create mode 100644 src/components/modal/helpers/modal-manager.js create mode 100644 src/mixins/listen-on-root.spec.js diff --git a/docs/plugins/bootstrap-vue.js b/docs/plugins/bootstrap-vue.js index ded2ebf784d..f6e69ddd063 100644 --- a/docs/plugins/bootstrap-vue.js +++ b/docs/plugins/bootstrap-vue.js @@ -1,7 +1,4 @@ import Vue from 'vue' import BootstrapVue from '../../src' -Vue.use(BootstrapVue, { - // Make sure modals in the docs are above anything else - BModal: { zIndexOffset: 2000 } -}) +Vue.use(BootstrapVue, {}) diff --git a/src/components/modal/helpers/modal-manager.js b/src/components/modal/helpers/modal-manager.js new file mode 100644 index 00000000000..a452bbe9cb1 --- /dev/null +++ b/src/components/modal/helpers/modal-manager.js @@ -0,0 +1,221 @@ +// +// private modalManager helper +// +// Handles controlling modal stacking zIndexes and body adjustments/classes +// +import Vue from 'vue' +import { inBrowser } from '../../../utils/env' +import { + getAttr, + hasAttr, + removeAttr, + setAttr, + addClass, + removeClass, + getBCR, + getCS, + selectAll, + requestAF +} from '../../../utils/dom' + +// Default modal backdrop z-index +const DEFAULT_ZINDEX = 1040 + +// Selectors for padding/margin adjustments +const Selector = { + FIXED_CONTENT: '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top', + STICKY_CONTENT: '.sticky-top', + NAVBAR_TOGGLER: '.navbar-toggler' +} + +const ModalManager = Vue.extend({ + data() { + return { + modals: [], + baseZIndex: null, + scrollbarWidth: null, + isBodyOverflowing: false + } + }, + computed: { + modalCount() { + return this.modals.length + }, + modalsAreOpen() { + return this.modalCount > 0 + } + }, + watch: { + modalCount(newCount, oldCount) { + if (inBrowser) { + this.getScrollbarWidth() + if (newCount > 0 && oldCount === 0) { + // Transitioning to modal(s) open + this.checkScrollbar() + this.setScrollbar() + addClass(document.body, 'modal-open') + } else if (newCount === 0 && oldCount > 0) { + // Transitioning to modal(s) closed + this.resetScrollbar() + removeClass(document.body, 'modal-open') + } + setAttr(document.body, 'data-modal-open-count', String(newCount)) + } + }, + modals(newVal, oldVal) { + this.checkScrollbar() + requestAF(() => { + this.updateModals(newVal || []) + }) + } + }, + methods: { + // Public methods + registerModal(modal) { + if (modal && this.modals.indexOf(modal) === -1) { + // Add modal to modals array + this.modals.push(modal) + modal.$once('hook:beforeDestroy', () => { + this.unregisterModal(modal) + }) + } + }, + unregisterModal(modal) { + const index = this.modals.indexOf(modal) + if (index > -1) { + // Remove modal from modals arary + this.modals.splice(index, 1) + // Reset the modal's data + if (!(modal._isBeingDestroyed || modal._isDestroyed)) { + this.resetModal(modal) + } + } + }, + getBaseZIndex() { + if (this.baseZIndex === null && inBrowser) { + // Create a temporary div.modal-backdrop to get computed z-index + const div = document.createElement('div') + div.className = 'modal-backdrop d-none' + div.style.display = 'none' + document.body.appendChild(div) + this.baseZIndex = parseInt(getCS(div).zIndex || DEFAULT_ZINDEX, 10) + document.body.removeChild(div) + } + return this.baseZIndex || DEFAULT_ZINDEX + }, + getScrollbarWidth() { + if (this.scrollbarWidth === null && inBrowser) { + // Create a temporary div.measure-scrollbar to get computed z-index + const div = document.createElement('div') + div.className = 'modal-scrollbar-measure' + document.body.appendChild(div) + this.scrollbarWidth = getBCR(div).width - div.clientWidth + document.body.removeChild(div) + } + return this.scrollbarWidth || 0 + }, + // Private methods + updateModals(modals) { + const baseZIndex = this.getBaseZIndex() + const scrollbarWidth = this.getScrollbarWidth() + modals.forEach((modal, index) => { + // We update data values on each modal + modal.zIndex = baseZIndex + index + modal.scrollbarWidth = scrollbarWidth + modal.isTop = index === this.modals.length - 1 + modal.isBodyOverflowing = this.isBodyOverflowing + }) + }, + resetModal(modal) { + if (modal) { + modal.zIndex = this.getBaseZIndex() + modal.isTop = true + modal.isBodyOverflowing = false + } + }, + checkScrollbar() { + // Determine if the body element is overflowing + // const { left, right, height } = getBCR(document.body) + // Extra check for body.height needed for stacked modals + // this.isBodyOverflowing = left + right < window.innerWidth || height > window.innerHeight + const { left, right } = getBCR(document.body) + this.isBodyOverflowing = left + right < window.innerWidth + }, + setScrollbar() { + const body = document.body + // Storage place to cache changes to margins and padding + // Note: This assumes the following element types are not added to the + // document after the modal has opened. + body._paddingChangedForModal = body._paddingChangedForModal || [] + body._marginChangedForModal = body._marginChangedForModal || [] + if (this.isBodyOverflowing) { + const scrollbarWidth = this.scrollbarWidth + // Adjust fixed content padding + /* istanbul ignore next: difficult to test in JSDOM */ + selectAll(Selector.FIXED_CONTENT).forEach(el => { + const actualPadding = el.style.paddingRight + const calculatedPadding = getCS(el).paddingRight || 0 + setAttr(el, 'data-padding-right', actualPadding) + el.style.paddingRight = `${parseFloat(calculatedPadding) + scrollbarWidth}px` + body._paddingChangedForModal.push(el) + }) + // Adjust sticky content margin + /* istanbul ignore next: difficult to test in JSDOM */ + selectAll(Selector.STICKY_CONTENT).forEach(el => { + const actualMargin = el.style.marginRight + const calculatedMargin = getCS(el).marginRight || 0 + setAttr(el, 'data-margin-right', actualMargin) + el.style.marginRight = `${parseFloat(calculatedMargin) - scrollbarWidth}px` + body._marginChangedForModal.push(el) + }) + // Adjust navbar-toggler margin + /* istanbul ignore next: difficult to test in JSDOM */ + selectAll(Selector.NAVBAR_TOGGLER).forEach(el => { + const actualMargin = el.style.marginRight + const calculatedMargin = getCS(el).marginRight || 0 + setAttr(el, 'data-margin-right', actualMargin) + el.style.marginRight = `${parseFloat(calculatedMargin) + scrollbarWidth}px` + body._marginChangedForModal.push(el) + }) + // Adjust body padding + const actualPadding = body.style.paddingRight + const calculatedPadding = getCS(body).paddingRight + setAttr(body, 'data-padding-right', actualPadding) + body.style.paddingRight = `${parseFloat(calculatedPadding) + scrollbarWidth}px` + } + }, + resetScrollbar() { + const body = document.body + if (body._paddingChangedForModal) { + // Restore fixed content padding + body._paddingChangedForModal.forEach(el => { + /* istanbul ignore next: difficult to test in JSDOM */ + if (hasAttr(el, 'data-padding-right')) { + el.style.paddingRight = getAttr(el, 'data-padding-right') || '' + removeAttr(el, 'data-padding-right') + } + }) + } + if (body._marginChangedForModal) { + // Restore sticky content and navbar-toggler margin + body._marginChangedForModal.forEach(el => { + /* istanbul ignore next: difficult to test in JSDOM */ + if (hasAttr(el, 'data-margin-right')) { + el.style.marginRight = getAttr(el, 'data-margin-right') || '' + removeAttr(el, 'data-margin-right') + } + }) + } + body._paddingChangedForModal = null + body._marginChangedForModal = null + // Restore body padding + if (hasAttr(body, 'data-padding-right')) { + body.style.paddingRight = getAttr(body, 'data-padding-right') || '' + removeAttr(body, 'data-padding-right') + } + } + } +}) + +// Export our Modal Manager +export default new ModalManager() diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js index af7268ffa28..65d72108f02 100644 --- a/src/components/modal/modal.js +++ b/src/components/modal/modal.js @@ -1,41 +1,21 @@ import BButton from '../button/button' import BButtonClose from '../button/button-close' +import modalManager from './helpers/modal-manager' import idMixin from '../../mixins/id' import listenOnRootMixin from '../../mixins/listen-on-root' import observeDom from '../../utils/observe-dom' import warn from '../../utils/warn' import KeyCodes from '../../utils/key-codes' import BvEvent from '../../utils/bv-event.class' +import { inBrowser } from '../../utils/env' import { getComponentConfig } from '../../utils/config' import { stripTags } from '../../utils/html' -import { - addClass, - contains, - eventOff, - eventOn, - getAttr, - getBCR, - getCS, - hasAttr, - hasClass, - isVisible, - removeAttr, - removeClass, - select, - selectAll, - setAttr -} from '../../utils/dom' +import { contains, eventOff, eventOn, isVisible, select } from '../../utils/dom' const NAME = 'BModal' -// Selectors for padding/margin adjustments -const Selector = { - FIXED_CONTENT: '.fixed-top, .fixed-bottom, .is-fixed, .sticky-top', - STICKY_CONTENT: '.sticky-top', - NAVBAR_TOGGLER: '.navbar-toggler' -} - -// ObserveDom config +// ObserveDom config to detect changes in modal content +// so that we can adjust the modal padding if needed const OBSERVER_CONFIG = { subtree: true, childList: true, @@ -44,34 +24,6 @@ const OBSERVER_CONFIG = { attributeFilter: ['style', 'class'] } -// Modal open count helpers -const getModalOpenCount = () => parseInt(getAttr(document.body, 'data-modal-open-count') || 0, 10) - -const setModalOpenCount = count => { - setAttr(document.body, 'data-modal-open-count', String(count)) - return count -} - -const incrementModalOpenCount = () => setModalOpenCount(getModalOpenCount() + 1) -const decrementModalOpenCount = () => setModalOpenCount(Math.max(getModalOpenCount() - 1, 0)) - -// Returns the current visible modal highest z-index -const getModalMaxZIndex = () => { - return ( - selectAll('div.modal') // Find all modals that are in document - .filter(isVisible) // Filter only visible ones - .map(m => m.parentElement) // Select the outer div - // Compute the highest z-index - .reduce((max, el) => Math.max(max, parseInt(el.style.zIndex || 0, 10)), 0) - ) -} - -const getModalZIndexOffset = () => getComponentConfig(NAME, 'zIndexOffset') - -// Returns the next z-index to be used by a modal to ensure proper -// stacking regardless of document order -const getModalNextZIndex = () => getModalMaxZIndex() + getModalZIndexOffset() - // @vue/component export default { name: NAME, @@ -269,17 +221,16 @@ export default { 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 + isModalOverflowing: false, + return_focus: this.returnFocus || null, + // The following items are controlled by the modalManager instance scrollbarWidth: 0, - zIndex: 0, // z-index for modal stacking - isTop: true, // If the modal is the topmost opened modal - isBodyOverflowing: false, - return_focus: this.returnFocus || null + zIndex: modalManager.getBaseZIndex(), + isTop: true, + isBodyOverflowing: false } }, computed: { - contentClasses() { - return ['modal-content', this.contentClass] - }, modalClasses() { return [ { @@ -290,6 +241,13 @@ export default { this.modalClass ] }, + modalStyles() { + const sbWidth = `${this.scrollbarWidth}px` + return { + paddingLeft: !this.isBodyOverflowing && this.isModalOverflowing ? sbWidth : '', + paddingRight: this.isBodyOverflowing && !this.isModalOverflowing ? sbWidth : '' + } + }, dialogClasses() { return [ { @@ -336,8 +294,8 @@ export default { ] }, modalOuterStyle() { + // Styles needed for proper stacking of modals return { - // We only set these styles on the stacked modals (z-index > 0) position: 'absolute', zIndex: this.zIndex } @@ -353,16 +311,14 @@ export default { created() { // Define non-reactive properties this._observer = null - // Set initial z-index defined in the config - this.zIndex = getModalZIndexOffset() }, mounted() { + // Set initial z-index as queried from the DOM + this.zIndex = modalManager.getBaseZIndex() // Listen for events from others to either open or close ourselves // and listen to all modals to enable/disable enforce focus this.listenOnRoot('bv::show::modal', this.showHandler) - this.listenOnRoot('bv::modal::shown', this.shownHandler) this.listenOnRoot('bv::hide::modal', this.hideHandler) - this.listenOnRoot('bv::modal::hidden', this.hiddenHandler) this.listenOnRoot('bv::toggle::modal', this.toggleHandler) // Listen for `bv:modal::show events`, and close ourselves if the // opening modal not us @@ -378,21 +334,12 @@ export default { this._observer.disconnect() this._observer = null } - // Ensure our root 'once' listener is gone - this.$root.$off('bv::modal::hidden', this.doShow) this.setEnforceFocus(false) this.setResizeEvent(false) if (this.is_visible) { this.is_visible = false this.is_show = false this.is_transitioning = false - const count = decrementModalOpenCount() - if (count === 0) { - // Re-adjust body/navbar/fixed padding/margins (as we were the last modal open) - this.setModalOpenClass(false) - this.resetScrollbar() - this.resetDialogAdjustments() - } } }, methods: { @@ -415,8 +362,9 @@ export default { cancelable: true, vueTarget: this, target: this.$refs.modal, - modalId: this.safeId(), - relatedTarget: null + relatedTarget: null, + // Modal specifi properties + modalId: this.safeId() }) this.emitEvent(showEvt) // Don't show if canceled @@ -424,14 +372,6 @@ export default { this.is_opening = false return } - if (!this.noStacking) { - // Find the z-index to use - this.zIndex = getModalNextZIndex() - } else if (hasClass(document.body, 'modal-open')) { - // If another modal is already open, wait for it to close - this.$root.$once('bv::modal::hidden', this.doShow) - return - } // Show the modal this.doShow() }, @@ -442,13 +382,13 @@ export default { } this.is_closing = true const hideEvt = new BvEvent('hide', { + // BvEvent standard properties cancelable: true, vueTarget: this, target: this.$refs.modal, - modalId: this.safeId(), - // This could be the trigger element/component reference relatedTarget: null, - isOK: trigger || null, + // Modal specific properties and methods + modalId: this.safeId(), trigger: trigger || null, cancel() /* istanbul ignore next */ { // Backwards compatibility @@ -456,10 +396,13 @@ export default { this.preventDefault() } }) + // We emit specific event for one of the three built-in buttons if (trigger === 'ok') { this.$emit('ok', hideEvt) } else if (trigger === 'cancel') { this.$emit('cancel', hideEvt) + } else if (trigger === 'headerclose') { + this.$emit('close', hideEvt) } this.emitEvent(hideEvt) // Hide if not canceled @@ -473,6 +416,7 @@ export default { this._observer = null } this.is_visible = false + // Update the v-model this.$emit('change', false) }, // Public method to toggle modal visibility @@ -488,6 +432,12 @@ export default { }, // Private method to finish showing modal doShow() { + /* istanbul ignore next: commenting out for now until we can test stacking */ + if (modalManager.modalsAreOpen && this.noStacking) { + // If another modal(s) is already open, wait for it(them) to close + this.listenOnRootOnce('bv::modal::hidden', this.doShow) + return + } // Place modal in DOM if lazy this.is_hidden = false this.$nextTick(() => { @@ -495,26 +445,21 @@ export default { // before we show it this.is_visible = true this.is_opening = false + // Update the v-model this.$emit('change', true) // Observe changes in modal content and adjust if necessary this._observer = observeDom( this.$refs.content, - this.adjustDialog.bind(this), + this.checkModalOverflow.bind(this), OBSERVER_CONFIG ) }) }, // Transition handlers onBeforeEnter() { - this.getScrollbarWidth() this.is_transitioning = true - this.checkScrollbar() - const count = incrementModalOpenCount() - if (count === 1) { - this.setScrollbar() - this.setModalOpenClass(true) - } - this.adjustDialog() + modalManager.registerModal(this) + this.checkModalOverflow() this.setResizeEvent(true) }, onEnter() { @@ -528,8 +473,8 @@ export default { cancelable: false, vueTarget: this, target: this.$refs.modal, - modalId: this.safeId(), - relatedTarget: null + relatedTarget: null, + modalId: this.safeId() }) this.emitEvent(shownEvt) this.focusFirst() @@ -546,34 +491,28 @@ export default { }, onAfterLeave() { this.is_block = false - this.resetDialogAdjustments() this.is_transitioning = false - const count = decrementModalOpenCount() - if (count === 0) { - this.resetScrollbar() - this.setModalOpenClass(false) - } this.setEnforceFocus(false) + this.isModalOverflowing = false this.$nextTick(() => { - this.is_hidden = this.lazy || false - this.zIndex = getModalZIndexOffset() this.returnFocusTo() this.is_closing = false const hiddenEvt = new BvEvent('hidden', { cancelable: false, vueTarget: this, target: this.lazy ? null : this.$refs.modal, - modalId: this.safeId(), - relatedTarget: null + relatedTarget: null, + modalId: this.safeId() }) this.emitEvent(hiddenEvt) + modalManager.unregisterModal(this) }) }, // Event emitter emitEvent(bvEvt) { const type = bvEvt.type this.$emit(type, bvEvt) - this.$root.$emit(`bv::modal::${type}`, bvEvt, this.safeId()) + this.emitOnRoot(`bv::modal::${type}`, bvEvt, bvEvt.modalId) }, // UI event handlers onClickOut(evt) { @@ -617,8 +556,9 @@ export default { setResizeEvent(on) { const options = { passive: true, capture: false } const method = on ? eventOn : eventOff - method(window, 'resize', this.adjustDialog, options) - method(window, 'orientationchange', this.adjustDialog, options) + // These events should probably also check if body is overflowing + method(window, 'resize', this.checkModalOverflow, options) + method(window, 'orientationchange', this.checkModalOverflow, options) }, // Root listener handlers showHandler(id, triggerEl) { @@ -637,42 +577,33 @@ export default { this.toggle(triggerEl) } }, - shownHandler() { - this.setTop() - }, - hiddenHandler() { - this.setTop() - }, - setTop() { - // Determine if we are the topmost visible modal - this.isTop = this.zIndex >= getModalMaxZIndex() - }, modalListener(bvEvt) { - // If another modal opens, close this one + // If another modal opens, close this one if stacking not permitted if (this.noStacking && bvEvt.vueTarget !== this) { this.hide() } }, // Focus control handlers focusFirst() { + // TODO: + // Add support for finding input element with 'autofocus' attribute set + // and focus that element // Don't try and focus if we are SSR - if (typeof document === 'undefined') { - /* istanbul ignore next */ - return - } - const modal = this.$refs.modal - const activeElement = document.activeElement - if (activeElement && contains(modal, activeElement)) { - // If `activeElement` is child of modal or is modal, no need to change focus - return - } - if (modal) { - // Make sure top of modal is showing (if longer than the viewport) - // and focus the modal content wrapper - this.$nextTick(() => { - modal.scrollTop = 0 - modal.focus() - }) + if (inBrowser) { + const modal = this.$refs.modal + const activeElement = document.activeElement + if (activeElement && contains(modal, activeElement)) { + // If `activeElement` is child of modal or is modal, no need to change focus + return + } + if (modal) { + // Make sure top of modal is showing (if longer than the viewport) + // and focus the modal content wrapper + this.$nextTick(() => { + modal.scrollTop = 0 + modal.focus() + }) + } } }, returnFocusTo() { @@ -689,122 +620,10 @@ export default { } } }, - // Utility methods - getScrollbarWidth() { - const scrollDiv = document.createElement('div') - scrollDiv.className = 'modal-scrollbar-measure' - document.body.appendChild(scrollDiv) - this.scrollbarWidth = getBCR(scrollDiv).width - scrollDiv.clientWidth - document.body.removeChild(scrollDiv) - }, - setModalOpenClass(open) { - const method = open ? addClass : removeClass - method(document.body, 'modal-open') - }, - adjustDialog() { - if (!this.is_visible) { - /* istanbul ignore next */ - return - } - const modal = this.$refs.modal - const isModalOverflowing = modal.scrollHeight > document.documentElement.clientHeight - /* istanbul ignore if: can't test in JSDOM */ - if (!this.isBodyOverflowing && isModalOverflowing) { - modal.style.paddingLeft = `${this.scrollbarWidth}px` - } else { - modal.style.paddingLeft = '' - } - /* istanbul ignore else: can't test in JSDOM */ - if (this.isBodyOverflowing && !isModalOverflowing) { - modal.style.paddingRight = `${this.scrollbarWidth}px` - } else { - modal.style.paddingRight = '' - } - }, - resetDialogAdjustments() { - const modal = this.$refs.modal - if (modal) { - modal.style.paddingLeft = '' - modal.style.paddingRight = '' - } - }, - checkScrollbar() { - const { left, right, height } = getBCR(document.body) - // Extra check for body.height needed for stacked modals - this.isBodyOverflowing = left + right < window.innerWidth || height > window.innerHeight - }, - setScrollbar() { - const body = document.body - // Storage place to cache changes to margins and padding - // Note: This assumes the following element types are not added - // to the document after the modal has opened. - body._paddingChangedForModal = body._paddingChangedForModal || [] - body._marginChangedForModal = body._marginChangedForModal || [] - /* istanbul ignore if: get Computed Style can't be tested in JSDOM */ - if (this.isBodyOverflowing) { - // Note: `DOMNode.style.paddingRight returns` the actual value or '' - // if not set while `$(DOMNode).css('padding-right')` returns the - // calculated value or 0 if not set - const scrollbarWidth = this.scrollbarWidth - // Adjust fixed content padding - selectAll(Selector.FIXED_CONTENT).forEach(el => { - const actualPadding = el.style.paddingRight - const calculatedPadding = getCS(el).paddingRight || 0 - setAttr(el, 'data-padding-right', actualPadding) - el.style.paddingRight = `${parseFloat(calculatedPadding) + scrollbarWidth}px` - body._paddingChangedForModal.push(el) - }) - // Adjust sticky content margin - selectAll(Selector.STICKY_CONTENT).forEach(el => { - const actualMargin = el.style.marginRight - const calculatedMargin = getCS(el).marginRight || 0 - setAttr(el, 'data-margin-right', actualMargin) - el.style.marginRight = `${parseFloat(calculatedMargin) - scrollbarWidth}px` - body._marginChangedForModal.push(el) - }) - // Adjust navbar-toggler margin - selectAll(Selector.NAVBAR_TOGGLER).forEach(el => { - const actualMargin = el.style.marginRight - const calculatedMargin = getCS(el).marginRight || 0 - setAttr(el, 'data-margin-right', actualMargin) - el.style.marginRight = `${parseFloat(calculatedMargin) + scrollbarWidth}px` - body._marginChangedForModal.push(el) - }) - // Adjust body padding - const actualPadding = body.style.paddingRight - const calculatedPadding = getCS(body).paddingRight - setAttr(body, 'data-padding-right', actualPadding) - body.style.paddingRight = `${parseFloat(calculatedPadding) + scrollbarWidth}px` - } - }, - resetScrollbar() { - const body = document.body - if (body._paddingChangedForModal) { - // Restore fixed content padding - body._paddingChangedForModal.forEach(el => { - /* istanbul ignore next: can't test in JSDOM */ - if (hasAttr(el, 'data-padding-right')) { - el.style.paddingRight = getAttr(el, 'data-padding-right') || '' - removeAttr(el, 'data-padding-right') - } - }) - } - if (body._marginChangedForModal) { - // Restore sticky content and navbar-toggler margin - body._marginChangedForModal.forEach(el => { - /* istanbul ignore next: can't test in JSDOM */ - if (hasAttr(el, 'data-margin-right')) { - el.style.marginRight = getAttr(el, 'data-margin-right') || '' - removeAttr(el, 'data-margin-right') - } - }) - } - body._paddingChangedForModal = null - body._marginChangedForModal = null - // Restore body padding - if (hasAttr(body, 'data-padding-right')) { - body.style.paddingRight = getAttr(body, 'data-padding-right') || '' - removeAttr(body, 'data-padding-right') + checkModalOverflow() { + if (this.is_visible) { + const modal = this.$refs.modal + this.isModalOverflowing = modal.scrollHeight > document.documentElement.clientHeight } } }, @@ -921,7 +740,8 @@ export default { 'div', { ref: 'content', - class: this.contentClasses, + staticClass: 'modal-content', + class: this.contentClass, attrs: { role: 'document', id: this.safeId('__BV_modal_content_'), @@ -947,6 +767,7 @@ export default { ref: 'modal', staticClass: 'modal', class: this.modalClasses, + style: this.modalStyles, directives: [ { name: 'show', rawName: 'v-show', value: this.is_visible, expression: 'is_visible' } ], diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js index 8e632d559e4..7fa54fe3992 100644 --- a/src/components/modal/modal.spec.js +++ b/src/components/modal/modal.spec.js @@ -1,9 +1,9 @@ import Modal from './modal' +import BvEvent from '../../utils/bv-event.class' import { mount, createWrapper } from '@vue/test-utils' -import { getComponentConfig } from '../../utils/config' -// Grab the configured base index -const CONFIG_ZINDEX = getComponentConfig('BModal', 'zIndexOffset') +// The defautl Z-INDEX for modal backdrop +const DEFAULT_ZINDEX = 1040 const waitAF = () => new Promise(resolve => requestAnimationFrame(resolve)) @@ -39,8 +39,6 @@ describe('modal', () => { } }) - expect(CONFIG_ZINDEX).toBeGreaterThan(0) - expect(wrapper.isVueInstance()).toBe(true) await wrapper.vm.$nextTick() @@ -54,7 +52,7 @@ describe('modal', () => { expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual(`${CONFIG_ZINDEX}`) + expect($outer.element.style.zIndex).toEqual(`${DEFAULT_ZINDEX}`) // Should not have a backdrop expect($outer.find('div.modal-backdrop').exists()).toBe(false) @@ -131,7 +129,7 @@ describe('modal', () => { expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual(`${CONFIG_ZINDEX}`) + expect($outer.element.style.zIndex).toEqual(`${DEFAULT_ZINDEX}`) // Main modal wrapper const $modal = $outer.find('div.modal') @@ -204,7 +202,7 @@ describe('modal', () => { expect($outer.is('div')).toBe(true) expect($outer.classes().length).toBe(0) expect($outer.element.style.position).toEqual('absolute') - expect($outer.element.style.zIndex).toEqual(`${CONFIG_ZINDEX}`) + expect($outer.element.style.zIndex).toEqual(`${DEFAULT_ZINDEX}`) // Main modal wrapper const $modal = $outer.find('div.modal') @@ -305,6 +303,7 @@ describe('modal', () => { it('header close button triggers modal close and is preventable', async () => { let cancelHide = true let trigger = null + let evt = null const wrapper = mount(Modal, { attachToDocument: true, stubs: { @@ -320,6 +319,7 @@ describe('modal', () => { bvEvent.preventDefault() } trigger = bvEvent.trigger + evt = bvEvent } } }) @@ -347,10 +347,12 @@ describe('modal', () => { expect(wrapper.emitted('hide')).not.toBeDefined() expect(trigger).toEqual(null) + expect(evt).toEqual(null) // Try and close modal (but we prevent it) $close.trigger('click') expect(trigger).toEqual('headerclose') + expect(evt).toBeInstanceOf(BvEvent) await wrapper.vm.$nextTick() await waitAF() @@ -363,8 +365,10 @@ describe('modal', () => { // Try and close modal (and not prevent it) cancelHide = false trigger = null + evt = null $close.trigger('click') expect(trigger).toEqual('headerclose') + expect(evt).toBeInstanceOf(BvEvent) await wrapper.vm.$nextTick() await waitAF() @@ -624,5 +628,74 @@ describe('modal', () => { wrapper.destroy() }) + + it('show event is cancellable', async () => { + let prevent = true + let called = 0 + const wrapper = mount(Modal, { + attachToDocument: true, + stubs: { + transition: false + }, + propsData: { + id: 'test', + visible: false + } + }) + + 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) + + expect($modal.element.style.display).toEqual('none') + + wrapper.vm.$on('show', bvEvt => { + called = true + if (prevent) { + bvEvt.preventDefault() + } + }) + + // Try and open modal via `bv::show::modal` + wrapper.vm.$root.$emit('bv::show::modal', 'test') + + await wrapper.vm.$nextTick() + await waitAF() + await wrapper.vm.$nextTick() + await waitAF() + + // Modal should not open + expect(called).toBe(true) + expect($modal.element.style.display).toEqual('none') + + await wrapper.vm.$nextTick() + await waitAF() + await wrapper.vm.$nextTick() + await waitAF() + + // Allow modal to open + prevent = false + called = false + + // Try and open modal via `bv::show::modal` + wrapper.vm.$root.$emit('bv::show::modal', 'test') + + await wrapper.vm.$nextTick() + await waitAF() + await wrapper.vm.$nextTick() + await waitAF() + + // Modal should now be open + expect(called).toBe(true) + expect($modal.element.style.display).toEqual('') + + wrapper.destroy() + }) }) }) diff --git a/src/mixins/listen-on-root.js b/src/mixins/listen-on-root.js index a4390ad78f6..42af1e50466 100644 --- a/src/mixins/listen-on-root.js +++ b/src/mixins/listen-on-root.js @@ -30,6 +30,30 @@ export default { return this }, + /** + * Safely register a $once event listener on the root Vue node. + * While Vue automatically removes listeners for individual components, + * when a component registers a listener on root and is destroyed, + * this orphans a callback because the node is gone, + * but the root does not clear the callback. + * + * When registering a $root listener, it also registers a listener on + * the component's `beforeDestroy` hook to automatically remove the + * event listener from the $root instance. + * + * @param {string} event + * @param {function} callback + * @chainable + */ + listenOnRootOnce(event, callback) { + this.$root.$once(event, callback) + this.$on('hook:beforeDestroy', () => { + this.$root.$off(event, callback) + }) + // Return this for easy chaining + return this + }, + /** * Convenience method for calling vm.$emit on vm.$root. * @param {string} event diff --git a/src/mixins/listen-on-root.spec.js b/src/mixins/listen-on-root.spec.js new file mode 100644 index 00000000000..2f084a45715 --- /dev/null +++ b/src/mixins/listen-on-root.spec.js @@ -0,0 +1,73 @@ +import listenOnRootMixin from './listen-on-root' +import { mount, createLocalVue as CreateLocalVue } from '@vue/test-utils' + +describe('mixins/listen-on-root', () => { + const localVue = new CreateLocalVue() + it('works', async () => { + const spyOn = jest.fn() + const spyOnce = jest.fn() + + const TestComponent = localVue.extend({ + mixins: [listenOnRootMixin], + created() { + this.listenOnRoot('root-on', spyOn) + this.listenOnRootOnce('root-once', spyOnce) + }, + render(h) { + return h('div', {}, this.$slots.default) + } + }) + + const App = localVue.extend({ + components: { TestComponent }, + props: { + destroy: { + type: Boolean, + default: false + } + }, + render(h) { + return h('div', {}, [this.destroy ? h(false) : h(TestComponent, {}, 'test-component')]) + } + }) + + const wrapper = mount(App, { + localVue: localVue, + propsData: { + destroy: false + } + }) + + expect(wrapper.isVueInstance()).toBe(true) + expect(wrapper.text()).toEqual('test-component') + + expect(spyOn).not.toHaveBeenCalled() + expect(spyOnce).not.toHaveBeenCalled() + + const $root = wrapper.vm.$root + + $root.$emit('root-on') + + expect(spyOn).toHaveBeenCalledTimes(1) + expect(spyOnce).not.toHaveBeenCalled() + + wrapper.setProps({ + destroy: true + }) + + expect(spyOn).toHaveBeenCalledTimes(1) + expect(spyOnce).not.toHaveBeenCalled() + + $root.$emit('root-on') + + expect(spyOn).toHaveBeenCalledTimes(1) + expect(spyOnce).not.toHaveBeenCalled() + + $root.$emit('root-once') + + expect(spyOn).toHaveBeenCalledTimes(1) + expect(spyOnce).not.toHaveBeenCalled() + + wrapper.destroy() + }) +}) diff --git a/src/utils/config.js b/src/utils/config.js index 6660af5575a..47493a509b5 100644 --- a/src/utils/config.js +++ b/src/utils/config.js @@ -76,8 +76,6 @@ const DEFAULTS = { blankColor: 'transparent' }, BModal: { - // Defaults to the Boostrap default for the modal backdrop - zIndexOffset: 1040, cancelTitle: 'Cancel', cancelVariant: 'secondary', okTitle: 'OK', diff --git a/src/utils/dom.js b/src/utils/dom.js index dd9ec1ea36f..ce35e82c61e 100644 --- a/src/utils/dom.js +++ b/src/utils/dom.js @@ -62,6 +62,7 @@ export const isVisible = el => { } if (el.style.display === 'none') { // We do this check to help with vue-test-utils when using v-show + /* istanbul ignore next */ return false } // All browsers support getBoundingClientRect(), except JSDOM as it returns all 0's for values :(