fix(modal): exclude document.body when determining return focus element (bootstrap-vue#3228)

tmorehouse · web-flow · commit 092ab2decbd7 · 2019-05-02T17:12:45.000-03:00
diff --git a/src/components/modal/modal.js b/src/components/modal/modal.js
@@ -386,16 +386,8 @@ export default Vue.extend({
         return
       }
       this.is_opening = true
-      // Note: On IE11, `document.activeElement` may be null. So we test it for
-      // truthyness first.
-      // https://github.com/bootstrap-vue/bootstrap-vue/issues/3206
-      if (isBrowser && document.activeElement && document.activeElement.focus) {
-        // Preset the fallback return focus value if it is not set
-        // `document.activeElement` should be the trigger element that was clicked or
-        // in the case of using the v-model, which ever element has current focus
-        // Will be overridden by some commands such as toggle, etc.
-        this.return_focus = this.return_focus || document.activeElement
-      }
+      // Set the element to return focus to when closed
+      this.return_focus = this.return_focus || this.getActiveElement()
       const showEvt = new BvModalEvent('show', {
         cancelable: true,
         vueTarget: this,
@@ -465,6 +457,25 @@ export default Vue.extend({
         this.show()
       }
     },
+    // Private method to get the current document active element
+    getActiveElement() {
+      if (isBrowser) {
+        const activeElement = document.activeElement
+        // Note: On IE11, `document.activeElement` may be null. So we test it for
+        // truthyness first.
+        // https://github.com/bootstrap-vue/bootstrap-vue/issues/3206
+        // Returning focus to document.body may cause unwanted scrolls, so we
+        // exclude setting focus on body
+        if (activeElement && activeElement !== document.body && activeElement.focus) {
+          // Preset the fallback return focus value if it is not set
+          // `document.activeElement` should be the trigger element that was clicked or
+          // in the case of using the v-model, which ever element has current focus
+          // Will be overridden by some commands such as toggle, etc.
+          return activeElement
+        }
+      }
+      return null
+    },
     // Private method to finish showing modal
     doShow() {
       /* istanbul ignore next: commenting out for now until we can test stacking */
@@ -631,7 +642,7 @@ export default Vue.extend({
     // Root listener handlers
     showHandler(id, triggerEl) {
       if (id === this.id) {
-        this.return_focus = triggerEl || document.activeElement || null
+        this.return_focus = triggerEl || this.getActiveElement()
         this.show()
       }
     },
@@ -658,7 +669,7 @@ export default Vue.extend({
       // Don't try and focus if we are SSR
       if (isBrowser) {
         const modal = this.$refs.modal
-        const activeElement = document.activeElement
+        const activeElement = this.getActiveElement()
         // If the modal contains the activeElement, we don't do anything
         if (modal && !(activeElement && contains(modal, activeElement))) {
           // Make sure top of modal is showing (if longer than the viewport)
@@ -673,7 +684,7 @@ export default Vue.extend({
     returnFocusTo() {
       // Prefer `returnFocus` prop over event specified
       // `return_focus` value
-      let el = this.returnFocus || this.return_focus || document.activeElement || null
+      let el = this.returnFocus || this.return_focus || null
       // Is el a string CSS selector?
       el = isString(el) ? select(el) : el
       if (el) {
diff --git a/src/components/modal/modal.spec.js b/src/components/modal/modal.spec.js
@@ -968,68 +968,6 @@ describe('modal', () => {
   describe('focus management', () => {
     const localVue = new CreateLocalVue()
 
-    it('returns focus to document.body when no return focus set and not using v-b-toggle', async () => {
-      // JSDOM won't focus the document unless it has a tab index
-      document.body.tabIndex = 0
-
-      const wrapper = mount(BModal, {
-        attachToDocument: true,
-        localVue: localVue,
-        stubs: {
-          transition: false
-        },
-        propsData: {
-          static: true,
-          id: 'test',
-          visible: false
-        }
-      })
-
-      expect(wrapper.isVueInstance()).toBe(true)
-
-      await waitNT(wrapper.vm)
-      await waitRAF()
-      await waitNT(wrapper.vm)
-      await waitRAF()
-
-      const $modal = wrapper.find('div.modal')
-      expect($modal.exists()).toBe(true)
-
-      expect($modal.element.style.display).toEqual('none')
-      expect(document.activeElement).toBe(document.body)
-
-      // Try and open modal via `.toggle()` method
-      wrapper.vm.toggle()
-
-      await waitNT(wrapper.vm)
-      await waitRAF()
-      await waitNT(wrapper.vm)
-      await waitRAF()
-      await waitNT(wrapper.vm)
-      await waitNT(wrapper.vm)
-
-      // Modal should now be open
-      expect($modal.element.style.display).toEqual('')
-      expect(document.activeElement).not.toBe(document.body)
-      expect(wrapper.element.contains(document.activeElement)).toBe(true)
-
-      // Try and close modal via `.toggle()` method
-      wrapper.vm.toggle()
-
-      await waitNT(wrapper.vm)
-      await waitRAF()
-      await waitNT(wrapper.vm)
-      await waitRAF()
-      await waitNT(wrapper.vm)
-      await waitNT(wrapper.vm)
-
-      // Modal should now be closed
-      expect($modal.element.style.display).toEqual('none')
-      expect(document.activeElement).toBe(document.body)
-
-      wrapper.destroy()
-    })
-
     it('returns focus to previous active element when return focus not set and not using v-b-toggle', async () => {
       const App = localVue.extend({
         render(h) {
@@ -1186,7 +1124,7 @@ describe('modal', () => {
       wrapper.destroy()
     })
 
-    it('if focus leave modal it returns to modal', async () => {
+    it('if focus leaves modal it returns to modal', async () => {
       const App = localVue.extend({
         render(h) {
           return h('div', {}, [
