Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix role click shim #347

Merged
merged 3 commits into from
Nov 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 15 additions & 40 deletions javascripts/govuk/shim-links-with-button-role.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
// javascript 'shim' to trigger the click event of element(s)
// when the space key is pressed.
//
// usage instructions:
// GOVUK.shimLinksWithButtonRole.init();
// Created since some Assistive Technologies (for example some Screenreaders)
Copy link
Contributor

@robinwhittleton robinwhittleton Nov 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true? As I understand it this isn’t about assistive tech but about normalising the behaviour of real buttons and links-styled-to-look-like-buttons for standard browsers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this was the reason we did this, since some AT are telling users to press space which at the moment does not work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cfq arbitration needed 🙂

Copy link
Contributor

@gemmaleigh gemmaleigh Nov 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm with @NickColley on this one.

Here's the the original thread.

@aduggin said:

This is quite a complex issue - requires a long blog post to explain it fully.

The short version
For links that have role="button" we need to add an event handler so that it can be activated with the space bar (https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_button_role)

The medium version
We have links and buttons that look the same so there is a consistent call to action for sighted users.
Unfortunately this creates inconsistency for screen reader users and speech recognition users.
To progress through a transaction they have to sometimes interact with a link and sometimes with a button.

Adding role="button" to links provides consistency as it makes screen readers and speech recognition software treat it as a button. This only works in some screen reader browser combinations and the latest version of Dragon Naturally speaking (13) that support role="button".

Buttons can be activated with the space bar, links can't. This behaviour needs to be added with javascript to a link with role="button" as it is not automatically added by adding role="button"

So, adding role="button" and an event handler for the space bar will only provide the required consistency to some assistive technology users.

In an ideal world links look like links and buttons look like buttons

The long version
In a blog post that I need to find time to write - but includes information about how screen reader users and dragon users interact with links and buttons and the scenarios of when people will be confused by our links and buttons looking the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a link to Alistair's comment for context

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gemmaleigh, that's the whole answer. We started with AT users but our investigation has shown us that browser vendors haven't implemented the functionality on purpose and this was the recommended solution.

Copy link

@cfq cfq Nov 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a video of our test on buttons vs links on Dragon NaturallySpeaking 13: https://www.youtube.com/watch?v=NO7u2tEbFQY

// Will tell a user to press space on a 'button', so this functionality needs to be shimmed
// See https://github.com/alphagov/govuk_elements/pull/272#issuecomment-233028270
//
// If you want to customise the shim you can pass in a custom configuration
// object with your own selector for the target elements and addional keyup
// codes if there becomes a need to do so. For example:
// GOVUK.shimLinksWithButtonRole.init({ selector: '[role="button"]' });
// Usage instructions:
// GOVUK.shimLinksWithButtonRole.init();
;(function (global) {
'use strict'

Expand All @@ -16,40 +15,16 @@

GOVUK.shimLinksWithButtonRole = {

// default configuration that can be overridden by passing object as second parameter to module
config: {
// the target element(s) to attach the shim event to
selector: 'a[role="button"]',
// array of keys to match against upon the keyup event
keycodes: [
32 // spacekey
]
},

// event behaviour (not a typical anonymous function for resuse if needed)
triggerClickOnTarget: function triggerClickOnTarget (event) {
// if the code from this event is in the keycodes array then
if ($.inArray(event.which, this.config.keycodes) !== -1) {
event.preventDefault()
// trigger the target's click event
event.target.click()
}
},

// By default this will find all links with role attribute set to
// 'button' and will trigger their click event when the space key (32) is pressed.
// @method init
// @param {Object} customConfig object to override default configuration
// {String} customConfig.selector a selector for the elements to be 'clicked'
// {Array} customConfig.keycodes an array of javascript keycode values to match against that when pressed will trigger the click
init: function init (customConfig) {
// extend the default config with any custom attributes passed in
this.config = $.extend(this.config, customConfig)
// if we have found elements then:
if ($(this.config.selector).length > 0) {
// listen to 'document' for keyup event on the elements and fire the triggerClickOnTarget
$(document).on('keyup', this.config.selector, this.triggerClickOnTarget.bind(this))
}
init: function init () {
// listen to 'document' for keydown event on the any elements that should be buttons.
$(document).on('keydown', '[role="button"]', function (event) {
// if the keyCode (which) is 32 it's a space, let's simulate a click.
if (event.which === 32) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be rewritten as:

var userPressedSpace = event.which === 32
if (userPressedSpace) {

Wouldn't need the comment then.

Copy link
Contributor

@fofr fofr Nov 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than the code comment I’d have written something like: var SPACEBAR = 32; and if (event.which === SPACEBAR) { etc.

event.preventDefault()
// trigger the target's click event
event.target.click()
}
})
}

}
Expand Down
14 changes: 7 additions & 7 deletions spec/unit/shim-links-with-button-role.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@ describe('shim-links-with-button-role', function () {
var GOVUK = window.GOVUK

var $buttonLink
var keyupEvent
var keyDownEvent

beforeEach(function () {
$buttonLink = $('<a role="button">Button</a>')
$buttonLink.on('click', function () {
$buttonLink.addClass('clicked')
})
$(document.body).append($buttonLink)
keyupEvent = $.Event('keyup')
keyupEvent.target = $buttonLink.get(0)
keyDownEvent = $.Event('keydown')
keyDownEvent.target = $buttonLink.get(0)
GOVUK.shimLinksWithButtonRole.init()
})

Expand All @@ -28,14 +28,14 @@ describe('shim-links-with-button-role', function () {
it('should trigger event on space', function () {
// Ideally we’d test the page loading functionality but that seems hard to
// do within a Jasmine context. Settle for checking a bound event trigger.
keyupEvent.which = 32 // Space character
$(document).trigger(keyupEvent)
keyDownEvent.which = 32 // Space character
$(document).trigger(keyDownEvent)
expect($buttonLink.hasClass('clicked')).toBe(true)
})

it('should not trigger event on tab', function () {
keyupEvent.which = 9 // Tab character
$(document).trigger(keyupEvent)
keyDownEvent.which = 9 // Tab character
$(document).trigger(keyDownEvent)
expect($buttonLink.hasClass('clicked')).toBe(false)
})
})