Skip to content
Permalink
Browse files
fix(b-button): when href is "#" add role=button and appropriate k…
…eydown handlers for A11Y (#4768)

* fix(b-button): when `href` is "#" add `role=button` and appropriate keydown handlers

* Update button.js

* Update button.spec.js

* Update button.spec.js

* Update button.spec.js

* Update button.spec.js

* Update button.js

* Update link.js

Co-authored-by: Jacob Müller <jacob.mueller.elz@gmail.com>
  • Loading branch information
tmorehouse and jacobmllr95 committed Feb 14, 2020
1 parent 97a65c2 commit 087a1283977061c44d5b059c203f13d2326dabae
Showing 3 changed files with 89 additions and 20 deletions.
@@ -1,5 +1,6 @@
import Vue from '../../utils/vue'
import { mergeData } from 'vue-functional-data-merge'
import KeyCodes from '../../utils/key-codes'
import pluckProps from '../../utils/pluck-props'
import { concat } from '../../utils/array'
import { getComponentConfig } from '../../utils/config'
@@ -47,8 +48,8 @@ const btnProps = {
default: false
},
pressed: {
// tri-state prop: true, false or null
// => on, off, not a toggle
// Tri-state: `true`, `false` or `null`
// => On, off, not a toggle
type: Boolean,
default: null
}
@@ -63,10 +64,11 @@ export const props = { ...linkProps, ...btnProps }

// --- Helper methods ---

// Returns true if a tag's name is name
// Returns `true` if a tag's name equals `name`
const tagIs = (tag, name) => toString(tag).toLowerCase() === toString(name).toLowerCase()

// Focus handler for toggle buttons. Needs class of 'focus' when focused.
// Focus handler for toggle buttons
// Needs class of 'focus' when focused
const handleFocus = evt => {
if (evt.type === 'focusin') {
addClass(evt.target, 'focus')
@@ -76,7 +78,7 @@ const handleFocus = evt => {
}

// Is the requested button a link?
// If tag prop is set to `a`, we use a b-link to get proper disabled handling
// If tag prop is set to `a`, we use a <b-link> to get proper disabled handling
const isLink = props => props.href || props.to || tagIs(props.tag, 'a')

// Is the button to be a toggle button?
@@ -109,31 +111,33 @@ const computeAttrs = (props, data) => {
const button = isButton(props)
const link = isLink(props)
const toggle = isToggle(props)
const nonStdTag = isNonStandardTag(props)
const nonStandardTag = isNonStandardTag(props)
const hashLink = link && props.href === '#'
const role = data.attrs && data.attrs.role ? data.attrs.role : null
let tabindex = data.attrs ? data.attrs.tabindex : null
if (nonStdTag) {
if (nonStandardTag || hashLink) {
tabindex = '0'
}
return {
// Type only used for "real" buttons
type: button && !link ? props.type : null,
// Disabled only set on "real" buttons
disabled: button ? props.disabled : null,
// We add a role of button when the tag is not a link or button for ARIA.
// Don't bork any role provided in data.attrs when isLink or isButton
role: nonStdTag ? 'button' : role,
// We set the aria-disabled state for non-standard tags
'aria-disabled': nonStdTag ? String(props.disabled) : null,
// We add a role of button when the tag is not a link or button for ARIA
// Don't bork any role provided in `data.attrs` when `isLink` or `isButton`
// Except when link has `href` of `#`
role: nonStandardTag || hashLink ? 'button' : role,
// We set the `aria-disabled` state for non-standard tags
'aria-disabled': nonStandardTag ? String(props.disabled) : null,
// For toggles, we need to set the pressed state for ARIA
'aria-pressed': toggle ? String(props.pressed) : null,
// autocomplete off is needed in toggle mode to prevent some browsers from
// remembering the previous setting when using the back button.
// `autocomplete="off"` is needed in toggle mode to prevent some browsers
// from remembering the previous setting when using the back button
autocomplete: toggle ? 'off' : null,
// Tab index is used when the component is not a button.
// `tabindex` is used when the component is not a button
// Links are tabbable, but don't allow disabled, while non buttons or links
// are not tabbable, so we mimic that functionality by disabling tabbing
// when disabled, and adding a tabindex of '0' to non buttons or non links.
// when disabled, and adding a `tabindex="0"` to non buttons or non links
tabindex: props.disabled && !button ? '-1' : tabindex
}
}
@@ -146,16 +150,33 @@ export const BButton = /*#__PURE__*/ Vue.extend({
render(h, { props, data, listeners, children }) {
const toggle = isToggle(props)
const link = isLink(props)
const nonStandardTag = isNonStandardTag(props)
const hashLink = link && props.href === '#'
const on = {
keydown(evt) {
// When the link is a `href="#"` or a non-standard tag (has `role="button"`),
// we add a keydown handlers for SPACE/ENTER
/* istanbul ignore next */
if (props.disabled || !(nonStandardTag || hashLink)) {
return
}
const { keyCode } = evt
// Add SPACE handler for `href="#"` and ENTER handler for non-standard tags
if (keyCode === KeyCodes.SPACE || (keyCode === KeyCodes.ENTER && nonStandardTag)) {
const target = evt.currentTarget || evt.target
evt.preventDefault()
target.click()
}
},
click(evt) {
/* istanbul ignore if: blink/button disabled should handle this */
if (props.disabled && isEvent(evt)) {
evt.stopPropagation()
evt.preventDefault()
} else if (toggle && listeners && listeners['update:pressed']) {
// Send .sync updates to any "pressed" prop (if .sync listeners)
// Concat will normalize the value to an array
// without double wrapping an array value in an array.
// Send `.sync` updates to any "pressed" prop (if `.sync` listeners)
// `concat()` will normalize the value to an array without
// double wrapping an array value in an array
concat(listeners['update:pressed']).forEach(fn => {
if (isFunction(fn)) {
fn(!props.pressed)
@@ -179,6 +179,22 @@ describe('button', () => {
// Actually returns 4, as disabled is there twice
expect(wrapper.attributes('aria-disabled')).toBeDefined()
expect(wrapper.attributes('aria-disabled')).toBe('true')
// Shouldnt have a role with href not `#`
expect(wrapper.attributes('role')).not.toEqual('button')
})

it('link with href="#" should have role="button"', async () => {
const wrapper = mount(BButton, {
propsData: {
href: '#'
}
})

expect(wrapper.is('a')).toBe(true)
expect(wrapper.classes()).toContain('btn')
expect(wrapper.classes()).toContain('btn-secondary')
expect(wrapper.classes()).not.toContain('disabled')
expect(wrapper.attributes('role')).toEqual('button')
})

it('should emit click event when clicked', async () => {
@@ -201,6 +217,38 @@ describe('button', () => {
expect(evt).toBeInstanceOf(MouseEvent)
})

it('link with href="#" should treat keydown.space as click', async () => {
let called = 0
let evt = null
const wrapper = mount(BButton, {
propsData: {
href: '#'
},
listeners: {
click: e => {
evt = e
called++
}
}
})

expect(wrapper.is('a')).toBe(true)
expect(wrapper.classes()).toContain('btn')
expect(wrapper.classes()).toContain('btn-secondary')
expect(wrapper.classes()).not.toContain('disabled')
expect(wrapper.attributes('role')).toEqual('button')

expect(called).toBe(0)
expect(evt).toEqual(null)

// We add keydown.space to make links act like buttons
wrapper.find('.btn').trigger('keydown.space')
expect(called).toBe(1)
expect(evt).toBeInstanceOf(Event)

// Links treat keydown.enter natively as a click
})

it('should not emit click event when clicked and disabled', async () => {
let called = 0
const wrapper = mount(BButton, {
@@ -168,7 +168,7 @@ export const BLink = /*#__PURE__*/ Vue.extend({
},
props: this.computedProps
}
// Add the event handlers. We must use `navtiveOn` for
// Add the event handlers. We must use `nativeOn` for
// `<router-link>`/`<nuxt-link>` instead of `on`
componentData[isRouterLink ? 'nativeOn' : 'on'] = {
// Transfer all listeners (native) to the root element

0 comments on commit 087a128

Please sign in to comment.