Skip to content

Commit

Permalink
fix(v-b-modal): ensure trigger element is keyboard accessible if not …
Browse files Browse the repository at this point in the history
…a link or button, for A11Y (#4365)
  • Loading branch information
tmorehouse committed Nov 11, 2019
1 parent fbbcffb commit f54ca29
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 7 deletions.
13 changes: 12 additions & 1 deletion src/components/modal/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 -->
15 changes: 11 additions & 4 deletions src/directives/modal/modal.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import KeyCodes from '../../utils/key-codes'
import {
eventOn,
eventOff,
Expand All @@ -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'
Expand All @@ -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')
}
}
}

Expand Down
50 changes: 48 additions & 2 deletions src/directives/modal/modal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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')
Expand Down Expand Up @@ -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
Expand All @@ -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')
Expand Down

0 comments on commit f54ca29

Please sign in to comment.