fix(v-b-modal): ensure trigger element is keyboard accessible if not a link or button, for A11Y (#4365)

tmorehouse · web-flow · commit f54ca2969a38 · 2019-11-11T13:48:28.000-04:00
diff --git a/src/components/modal/README.md b/src/components/modal/README.md
@@ -1020,7 +1020,7 @@ emitted.
 `<b-modal>` provides several accessibility features, including auto focus, return focus, keyboard
 (tab) _focus containment_, and automated `aria-*` attributes.
 
-### ARIA attributes
+### Modal ARIA attributes
 
 The `aria-labelledby` and `aria-describedby` attributes will appear on the modal automatically in
 most cases.
@@ -1158,4 +1158,15 @@ content and can make some of your elements unreachable via keyboard navigation.
 In some circumstances, you may need to disable the enforce focus feature. You can do this by setting
 the prop `no-enforce-focus`, although this is highly discouraged.
 
+### `v-b-modal` directive accessibility
+
+Notes on `v-b-modal` directive accessibility:
+
+- If the element is anything other than a `<button>` (or component that renders a `<button>`), the
+  ARIA `role` will be set to `button`, and a keydown event listeners for <kbd>ENTER</kbd> and
+  <kbd>SPACE</kbd> will be added, along with a `click` listener.
+- If the element is anything other than a `<button>` or `<a>` (or a component that renders either),
+  then a `tabindex` of `0` will be added to the element to ensure accessibility, unless there is
+  already a `tabindex` set.
+
 <!-- Component reference added automatically from component package.json -->
diff --git a/src/directives/modal/modal.js b/src/directives/modal/modal.js
@@ -1,3 +1,4 @@
+import KeyCodes from '../../utils/key-codes'
 import {
   eventOn,
   eventOff,
@@ -10,7 +11,6 @@ import {
 } from '../../utils/dom'
 import { isString } from '../../utils/inspect'
 import { keys } from '../../utils/object'
-import KeyCodes from '../../utils/key-codes'
 
 // Emitted show event for modal
 const EVENT_SHOW = 'bv::show::modal'
@@ -32,9 +32,16 @@ const getTriggerElement = el => {
 }
 
 const setRole = trigger => {
-  // Only set a role if the trigger element doesn't have one
-  if (trigger && trigger.tagName !== 'BUTTON' && !hasAttr(trigger, 'role')) {
-    setAttr(trigger, 'role', 'button')
+  // Ensure accessibility on non button elements
+  if (trigger && trigger.tagName !== 'BUTTON') {
+    // Only set a role if the trigger element doesn't have one
+    if (!hasAttr(trigger, 'role')) {
+      setAttr(trigger, 'role', 'button')
+    }
+    // Add a tabindex is not a button or link, and tabindex is not provided
+    if (trigger.tagName !== 'A' && !hasAttr(trigger, 'tabindex')) {
+      setAttr(trigger, 'tabindex', '0')
+    }
   }
 }
 
diff --git a/src/directives/modal/modal.spec.js b/src/directives/modal/modal.spec.js
@@ -26,6 +26,8 @@ describe('v-b-modal directive', () => {
 
     expect(wrapper.isVueInstance()).toBe(true)
     expect(wrapper.is('button')).toBe(true)
+    expect(wrapper.find('button').attributes('tabindex')).not.toBeDefined()
+    expect(wrapper.find('button').attributes('role')).not.toBeDefined()
     expect(spy).not.toHaveBeenCalled()
 
     const $button = wrapper.find('button')
@@ -36,6 +38,48 @@ describe('v-b-modal directive', () => {
     wrapper.destroy()
   })
 
+  it('works on links', async () => {
+    const localVue = new CreateLocalVue()
+    const spy = jest.fn()
+
+    const App = localVue.extend({
+      directives: {
+        bModal: VBModal
+      },
+      data() {
+        return {
+          text: 'link'
+        }
+      },
+      mounted() {
+        this.$root.$on(EVENT_SHOW, spy)
+      },
+      beforeDestroy() {
+        this.$root.$off(EVENT_SHOW, spy)
+      },
+      template: '<a href="#" v-b-modal.test>{{ text }}</a>'
+    })
+    const wrapper = mount(App, {
+      localVue: localVue
+    })
+
+    expect(wrapper.isVueInstance()).toBe(true)
+    expect(wrapper.is('a')).toBe(true)
+    expect(spy).not.toHaveBeenCalled()
+    expect(wrapper.find('a').attributes('role')).toBe('button')
+    expect(wrapper.find('a').attributes('tabindex')).not.toBeDefined()
+    expect(wrapper.find('a').text()).toBe('link')
+
+    const $link = wrapper.find('a')
+    $link.trigger('click')
+    expect(spy).toHaveBeenCalledTimes(1)
+    expect(spy).toBeCalledWith('test', $link.element)
+    expect(wrapper.find('a').attributes('role')).toBe('button')
+    expect(wrapper.find('a').attributes('tabindex')).not.toBeDefined()
+
+    wrapper.destroy()
+  })
+
   it('works on non-buttons', async () => {
     const localVue = new CreateLocalVue()
     const spy = jest.fn()
@@ -55,7 +99,7 @@ describe('v-b-modal directive', () => {
       beforeDestroy() {
         this.$root.$off(EVENT_SHOW, spy)
       },
-      template: '<span tabindex="0" v-b-modal.test>{{ text }}</span>'
+      template: '<span v-b-modal.test>{{ text }}</span>'
     })
     const wrapper = mount(App, {
       localVue: localVue
@@ -65,6 +109,7 @@ describe('v-b-modal directive', () => {
     expect(wrapper.is('span')).toBe(true)
     expect(spy).not.toHaveBeenCalled()
     expect(wrapper.find('span').attributes('role')).toBe('button')
+    expect(wrapper.find('span').attributes('tabindex')).toBe('0')
     expect(wrapper.find('span').text()).toBe('span')
 
     const $span = wrapper.find('span')
@@ -102,7 +147,7 @@ describe('v-b-modal directive', () => {
       beforeDestroy() {
         this.$root.$off(EVENT_SHOW, spy)
       },
-      template: '<span tabindex="0" v-b-modal.test>{{ text }}</span>'
+      template: '<span v-b-modal.test>{{ text }}</span>'
     })
     const wrapper = mount(App, {
       localVue: localVue
@@ -112,6 +157,7 @@ describe('v-b-modal directive', () => {
     expect(wrapper.is('span')).toBe(true)
     expect(spy).not.toHaveBeenCalled()
     expect(wrapper.find('span').attributes('role')).toBe('button')
+    expect(wrapper.find('span').attributes('tabindex')).toBe('0')
     expect(wrapper.find('span').text()).toBe('span')
 
     const $span = wrapper.find('span')
