Skip to content

Commit 087a128

Browse files
tmorehousejacobmllr95
tmorehouse
and
jacobmllr95
authored
fix(b-button): when href is "#" add role=button and appropriate keydown 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>
1 parent 97a65c2 commit 087a128

File tree

3 files changed

+89
-20
lines changed

3 files changed

+89
-20
lines changed

‎src/components/button/button.js

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Vue from '../../utils/vue'
22
import { mergeData } from 'vue-functional-data-merge'
3+
import KeyCodes from '../../utils/key-codes'
34
import pluckProps from '../../utils/pluck-props'
45
import { concat } from '../../utils/array'
56
import { getComponentConfig } from '../../utils/config'
@@ -47,8 +48,8 @@ const btnProps = {
4748
default: false
4849
},
4950
pressed: {
50-
// tri-state prop: true, false or null
51-
// => on, off, not a toggle
51+
// Tri-state: `true`, `false` or `null`
52+
// => On, off, not a toggle
5253
type: Boolean,
5354
default: null
5455
}
@@ -63,10 +64,11 @@ export const props = { ...linkProps, ...btnProps }
6364

6465
// --- Helper methods ---
6566

66-
// Returns true if a tag's name is name
67+
// Returns `true` if a tag's name equals `name`
6768
const tagIs = (tag, name) => toString(tag).toLowerCase() === toString(name).toLowerCase()
6869

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

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

8284
// Is the button to be a toggle button?
@@ -109,31 +111,33 @@ const computeAttrs = (props, data) => {
109111
const button = isButton(props)
110112
const link = isLink(props)
111113
const toggle = isToggle(props)
112-
const nonStdTag = isNonStandardTag(props)
114+
const nonStandardTag = isNonStandardTag(props)
115+
const hashLink = link && props.href === '#'
113116
const role = data.attrs && data.attrs.role ? data.attrs.role : null
114117
let tabindex = data.attrs ? data.attrs.tabindex : null
115-
if (nonStdTag) {
118+
if (nonStandardTag || hashLink) {
116119
tabindex = '0'
117120
}
118121
return {
119122
// Type only used for "real" buttons
120123
type: button && !link ? props.type : null,
121124
// Disabled only set on "real" buttons
122125
disabled: button ? props.disabled : null,
123-
// We add a role of button when the tag is not a link or button for ARIA.
124-
// Don't bork any role provided in data.attrs when isLink or isButton
125-
role: nonStdTag ? 'button' : role,
126-
// We set the aria-disabled state for non-standard tags
127-
'aria-disabled': nonStdTag ? String(props.disabled) : null,
126+
// We add a role of button when the tag is not a link or button for ARIA
127+
// Don't bork any role provided in `data.attrs` when `isLink` or `isButton`
128+
// Except when link has `href` of `#`
129+
role: nonStandardTag || hashLink ? 'button' : role,
130+
// We set the `aria-disabled` state for non-standard tags
131+
'aria-disabled': nonStandardTag ? String(props.disabled) : null,
128132
// For toggles, we need to set the pressed state for ARIA
129133
'aria-pressed': toggle ? String(props.pressed) : null,
130-
// autocomplete off is needed in toggle mode to prevent some browsers from
131-
// remembering the previous setting when using the back button.
134+
// `autocomplete="off"` is needed in toggle mode to prevent some browsers
135+
// from remembering the previous setting when using the back button
132136
autocomplete: toggle ? 'off' : null,
133-
// Tab index is used when the component is not a button.
137+
// `tabindex` is used when the component is not a button
134138
// Links are tabbable, but don't allow disabled, while non buttons or links
135139
// are not tabbable, so we mimic that functionality by disabling tabbing
136-
// when disabled, and adding a tabindex of '0' to non buttons or non links.
140+
// when disabled, and adding a `tabindex="0"` to non buttons or non links
137141
tabindex: props.disabled && !button ? '-1' : tabindex
138142
}
139143
}
@@ -146,16 +150,33 @@ export const BButton = /*#__PURE__*/ Vue.extend({
146150
render(h, { props, data, listeners, children }) {
147151
const toggle = isToggle(props)
148152
const link = isLink(props)
153+
const nonStandardTag = isNonStandardTag(props)
154+
const hashLink = link && props.href === '#'
149155
const on = {
156+
keydown(evt) {
157+
// When the link is a `href="#"` or a non-standard tag (has `role="button"`),
158+
// we add a keydown handlers for SPACE/ENTER
159+
/* istanbul ignore next */
160+
if (props.disabled || !(nonStandardTag || hashLink)) {
161+
return
162+
}
163+
const { keyCode } = evt
164+
// Add SPACE handler for `href="#"` and ENTER handler for non-standard tags
165+
if (keyCode === KeyCodes.SPACE || (keyCode === KeyCodes.ENTER && nonStandardTag)) {
166+
const target = evt.currentTarget || evt.target
167+
evt.preventDefault()
168+
target.click()
169+
}
170+
},
150171
click(evt) {
151172
/* istanbul ignore if: blink/button disabled should handle this */
152173
if (props.disabled && isEvent(evt)) {
153174
evt.stopPropagation()
154175
evt.preventDefault()
155176
} else if (toggle && listeners && listeners['update:pressed']) {
156-
// Send .sync updates to any "pressed" prop (if .sync listeners)
157-
// Concat will normalize the value to an array
158-
// without double wrapping an array value in an array.
177+
// Send `.sync` updates to any "pressed" prop (if `.sync` listeners)
178+
// `concat()` will normalize the value to an array without
179+
// double wrapping an array value in an array
159180
concat(listeners['update:pressed']).forEach(fn => {
160181
if (isFunction(fn)) {
161182
fn(!props.pressed)

‎src/components/button/button.spec.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,22 @@ describe('button', () => {
179179
// Actually returns 4, as disabled is there twice
180180
expect(wrapper.attributes('aria-disabled')).toBeDefined()
181181
expect(wrapper.attributes('aria-disabled')).toBe('true')
182+
// Shouldnt have a role with href not `#`
183+
expect(wrapper.attributes('role')).not.toEqual('button')
184+
})
185+
186+
it('link with href="#" should have role="button"', async () => {
187+
const wrapper = mount(BButton, {
188+
propsData: {
189+
href: '#'
190+
}
191+
})
192+
193+
expect(wrapper.is('a')).toBe(true)
194+
expect(wrapper.classes()).toContain('btn')
195+
expect(wrapper.classes()).toContain('btn-secondary')
196+
expect(wrapper.classes()).not.toContain('disabled')
197+
expect(wrapper.attributes('role')).toEqual('button')
182198
})
183199

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

220+
it('link with href="#" should treat keydown.space as click', async () => {
221+
let called = 0
222+
let evt = null
223+
const wrapper = mount(BButton, {
224+
propsData: {
225+
href: '#'
226+
},
227+
listeners: {
228+
click: e => {
229+
evt = e
230+
called++
231+
}
232+
}
233+
})
234+
235+
expect(wrapper.is('a')).toBe(true)
236+
expect(wrapper.classes()).toContain('btn')
237+
expect(wrapper.classes()).toContain('btn-secondary')
238+
expect(wrapper.classes()).not.toContain('disabled')
239+
expect(wrapper.attributes('role')).toEqual('button')
240+
241+
expect(called).toBe(0)
242+
expect(evt).toEqual(null)
243+
244+
// We add keydown.space to make links act like buttons
245+
wrapper.find('.btn').trigger('keydown.space')
246+
expect(called).toBe(1)
247+
expect(evt).toBeInstanceOf(Event)
248+
249+
// Links treat keydown.enter natively as a click
250+
})
251+
204252
it('should not emit click event when clicked and disabled', async () => {
205253
let called = 0
206254
const wrapper = mount(BButton, {

‎src/components/link/link.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ export const BLink = /*#__PURE__*/ Vue.extend({
168168
},
169169
props: this.computedProps
170170
}
171-
// Add the event handlers. We must use `navtiveOn` for
171+
// Add the event handlers. We must use `nativeOn` for
172172
// `<router-link>`/`<nuxt-link>` instead of `on`
173173
componentData[isRouterLink ? 'nativeOn' : 'on'] = {
174174
// Transfer all listeners (native) to the root element

0 commit comments

Comments
 (0)