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

Fix role click shim #347

merged 3 commits into from
Nov 22, 2016

Conversation

NickColley
Copy link
Contributor

When trying to implement the shim I found the page will scroll when pressing space, then you navigate to the page.

To fix this we need to instead listen for keydown and not keyup so we can prevent the unwanted scroll behaviour.

I've also added a commit to simplify the shim since there's not a clear need for this to be flexible.

With keyup we can't prevent scroll of the page when pressing space
so if we use keydown instead this will allow us to stop the scroll
with the expected behaviour
Before we had options to customize the shim when there doesn't
seem to be the need

This commit simplifies the shim to enable the space to click behaviour on all elements
with role="button"
// 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.

@@ -1,13 +1,11 @@
// 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

@robinwhittleton
Copy link
Contributor

I thought I remembered something about keydown being a bit weird in old IE, but I think I was actually thinking about keypress. I guess this has been tested in IE8+? If so I’ll merge.

@NickColley
Copy link
Contributor Author

Okay after testing this again I realise keyup is the right behaviour for space.

So this PR needs to be changed to handle keydown to prevent scrolling of the page, and handle keyup to click the button...

@NickColley
Copy link
Contributor Author

NickColley commented Nov 15, 2016

I can't find a place where that behaviour is defined in any spec, so I'm presuming it's down to the operating system.

I've tested this PR in IE8,IE9,IE10 and it works.

So this should be good to go as long as we can stomach the spacebar submitting on keydown and not keyup.

@robinwhittleton robinwhittleton merged commit e882086 into master Nov 22, 2016
@robinwhittleton robinwhittleton deleted the fix-role-click-shim branch November 22, 2016 10:15
robinwhittleton pushed a commit that referenced this pull request Nov 22, 2016
- Fix role="button" click shim (PR #347)
- Make font variables lowercase (PR #348)
- Update selection button documentation (PR #350)
@robinwhittleton robinwhittleton mentioned this pull request Nov 22, 2016
gemmaleigh added a commit to alphagov/govuk-prototype-kit that referenced this pull request Nov 22, 2016
https://raw.githubusercontent.com/alphagov/govuk_frontend_toolkit/master
/CHANGELOG.md

# 5.0.1

- Fix role="button" click shim ([PR
#347](alphagov/govuk_frontend_toolkit#347))
- Make font variables lowercase ([PR
#348](alphagov/govuk_frontend_toolkit#348))
- Update selection button documentation ([PR
#350](alphagov/govuk_frontend_toolkit#350))
gemmaleigh added a commit to alphagov/govuk_elements that referenced this pull request Nov 22, 2016
Update govuk_frontend_toolkit to 5.0.1
https://raw.githubusercontent.com/alphagov/govuk_frontend_toolkit/master
/CHANGELOG.md

# 5.0.1

- Fix role="button" click shim ([PR
#347](alphagov/govuk_frontend_toolkit#347))
- Make font variables lowercase ([PR
#348](alphagov/govuk_frontend_toolkit#348))
- Update selection button documentation ([PR
#350](alphagov/govuk_frontend_toolkit#350))
robinwhittleton pushed a commit to alphagov/verify-frontend that referenced this pull request Nov 29, 2016
Guntrisoft added a commit to guidance-guarantee-programme/pension_guidance that referenced this pull request Dec 2, 2016
- Removed phase-banner scss file as no longer needed

Full list of changes:

SelectionButtons will add a class to the label with the type of the child input
alphagov/govuk_frontend_toolkit#317

Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes
alphagov/govuk_frontend_toolkit#315

Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript
alphagov/govuk_frontend_toolkit#323

Remove unnecessary print font fallback that causes regression downstream
alphagov/govuk_frontend_toolkit#328

Update tooling to use npm script rather than grunt-shell
alphagov/govuk_frontend_toolkit#327

For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim).
alphagov/govuk_frontend_toolkit#329

Lint codebase using standard
alphagov/govuk_frontend_toolkit#334

Add semicolons at the start of IIFE's
alphagov/govuk_frontend_toolkit#339

Removal of external link styles and icons, if you are using the external-link-* mixins you will need to remove them from your codebase
alphagov/govuk_frontend_toolkit#293

Correct spelling of the 'accordion' icon, you will need to check for the incorrect spelling 'accordian' and update if you are using this icon
alphagov/govuk_frontend_toolkit#345

Amend GOVUK.StickAtTopWhenScrolling to resize the sticky element and shim when the .js-sticky-resize class is set
alphagov/govuk_frontend_toolkit#343

Allow custom options in GOVUK.analytics.trackPageview
alphagov/govuk_frontend_toolkit#332

Fix role="button" click shim
alphagov/govuk_frontend_toolkit#347

Make font variables lowercase
alphagov/govuk_frontend_toolkit#348

Update selection button documentation
alphagov/govuk_frontend_toolkit#350

Change colour used in phase tags to govuk-blue
alphagov/govuk_frontend_toolkit#353
Guntrisoft added a commit to guidance-guarantee-programme/pension_guidance that referenced this pull request Dec 2, 2016
- Removed phase-banner scss file as no longer needed

Full list of changes:

SelectionButtons will add a class to the label with the type of the child input
alphagov/govuk_frontend_toolkit#317

Add GOVUK.ShowHideContent JavaScript to support showing and hiding content, toggled by radio buttons and checkboxes
alphagov/govuk_frontend_toolkit#315

Fix error in IE - remove trailing comma from shimLinksWithButtonRole JavaScript
alphagov/govuk_frontend_toolkit#323

Remove unnecessary print font fallback that causes regression downstream
alphagov/govuk_frontend_toolkit#328

Update tooling to use npm script rather than grunt-shell
alphagov/govuk_frontend_toolkit#327

For smaller screens (<768px) ensure that the GOVUK.StickAtTopWhenScrolling JS "unsticks" the element which was previously "stuck" (by removing both the class which sets fixed positioning and the shim).
alphagov/govuk_frontend_toolkit#329

Lint codebase using standard
alphagov/govuk_frontend_toolkit#334

Add semicolons at the start of IIFE's
alphagov/govuk_frontend_toolkit#339

Removal of external link styles and icons, if you are using the external-link-* mixins you will need to remove them from your codebase
alphagov/govuk_frontend_toolkit#293

Correct spelling of the 'accordion' icon, you will need to check for the incorrect spelling 'accordian' and update if you are using this icon
alphagov/govuk_frontend_toolkit#345

Amend GOVUK.StickAtTopWhenScrolling to resize the sticky element and shim when the .js-sticky-resize class is set
alphagov/govuk_frontend_toolkit#343

Allow custom options in GOVUK.analytics.trackPageview
alphagov/govuk_frontend_toolkit#332

Fix role="button" click shim
alphagov/govuk_frontend_toolkit#347

Make font variables lowercase
alphagov/govuk_frontend_toolkit#348

Update selection button documentation
alphagov/govuk_frontend_toolkit#350

Change colour used in phase tags to govuk-blue
alphagov/govuk_frontend_toolkit#353
paulmsmith pushed a commit to dwp/pyca-prototype that referenced this pull request May 2, 2017
* adds addtional step for users to run npm install post updating via git

* Replace encrypted GitHub token

This updates the environment variable in the config to enable
deployment via Travis CI.

* change node version from 4 to 6

* Setting up a very simple test suite to check the server and build tasks run across nodejs 4, 5 & 6

* make mocha exit properly

* As npm test runs gulp test, remove this from travis.yml

* Simplify the sanity test check

App doesn't need to listen in the test script as supertest accepts the
app variable and handles the listening and un-listening itself.

This also removes the needs for the after block to stop the server.

* moved devdependencies to fix heroku

* Update govuk-elements-sass to 2.2.0

This release includes the custom radio button and checkbox styles.

https://github.com/alphagov/govuk_elements/blob/master/CHANGELOG.md#220

* Update readme to point to docs app

* Remove console log for the file path

This can be seen when viewing the docs app locally.

* v5.0.0

* Update govuk_frontend_toolkit to 5.0.1

https://raw.githubusercontent.com/alphagov/govuk_frontend_toolkit/master
/CHANGELOG.md

# 5.0.1

- Fix role="button" click shim ([PR
#347](alphagov/govuk_frontend_toolkit#347))
- Make font variables lowercase ([PR
#348](alphagov/govuk_frontend_toolkit#348))
- Update selection button documentation ([PR
#350](alphagov/govuk_frontend_toolkit#350))

* Use new font stack reference for unbranded templates

govuk_frontend_toolkit v5.0.1 lowercases the font stack names.

* google analytics

* only add google analytics in promomode

* Move analytics to separate include and use env variable

* Update ‘Back’ link URL

* Bump govuk-elements-sass to 2.2.1

This makes the phase tag in the phase banner $govuk-blue.

As the phase banner is now the same for both alpha and beta banners,
change the phase banner includes to use the recommended class
.phase-banner.

* Set travis to update the latest release branch after deployment

It looks like Travis is deploying the latest-release branch to Heroku.

Let’s move the script that checks for a version tag to after_deploy, in
the hope that it won’t affect the deployment of master to Heroku on a
successful build.

* Fix the "back" link to tutorials and examples

* Bump govuk template to 0.19.2

# 0.19.2

- Increase skiplink colour contrast ([PR
#263](alphagov/govuk_template#263))

# 0.19.1

- Have focus outline appear outside of element rather than covering it
in Safari and Chrome ([PR
#259](alphagov/govuk_template#259))

* Bump standard to current version

And fix reported issues: just spacing.

* Bump toolkit and nunjucks to latest versions

* Remove some unused packages

- consolidate was added to help replace mu2 for mustache rendering
- hogan.js was also in there for mustache rendering

* v5.0.1

- #330 Update GOV.UK toolkit and StandardJS to latest
- #328 Update GOV.UK template to latest
- #324 Fix the example question page’s back link

* use app.locals instead of app.use

* add git step to heroku guide

* Remove full stops from headings

* fix css sourcemaps

* Remove the title attribute

To match the change made to GOV.UK elements: alphagov/govuk_elements#389

* removing links to route.js and update path and file names in branching.html

* Prevent asking users to authenticate twice

This solves the problem with prototypes asking for username/password
twice.

The problem is caused by the ordering in the middleware.

When a user types in the URL for a prototype like
http://govuk-tagging.herokuapp.com they're asked for a
username/password first (via `utils.basicAuth`).

After filling that in they'll be able to "proceed" to the next step,
which redirects them to the `https://` version (via
`utils.forceHttps`).

Because the auth isn't shared between `http` and `https` version, they
are not authenticated anymore and have to enter their username/password
again.

Validated with https://test-prevent-double-auth.herokuapp.com/

* add override service name example

* add comment

* fix docs page

* bump gulp-sass to increase node-sass dependency to enable it to work with node 6 on linux

* add test for docs page

* allow search indexing in promo mode

* v5.1.0

* fix download link

* fix docs links

* Updating gulp-sass to 3.1.0

This is the latest version of gulp-sass.
https://www.npmjs.com/package/gulp-sass

* Bump govuk_frontend_toolkit to 5.1.1

# 5.1.1

- Update the alpha, beta and discovery colours to $govuk-blue to match
the updated phase banner ([PR
#370](alphagov/govuk_frontend_toolkit#370))
- Fix radio button show/hide behaviour when used outside a form ([PR
#375](alphagov/govuk_frontend_toolkit#375))
- Fix a "Stick at top when scrolling" component bug related to scroll
anchoring in Chrome 56+ ([PR
#376](alphagov/govuk_frontend_toolkit#376))
- Minor travis fixes ([PR
#373](alphagov/govuk_frontend_toolkit#373))

# 5.1.0

- Allow custom options when tracking events ([PR
#365](alphagov/govuk_frontend_toolkit#365))

# 5.0.3

- Change HMRC and DEFRA text colours for improved contrast ([PR
#368](alphagov/govuk_frontend_toolkit#368))

* formatting, clarify use of terminal, fix numbering

* Update govuk-elements-sass to 3.0.1

Changelog for GOV.UK elements:
https://github.com/alphagov/govuk_elements/blob/master/CHANGELOG.md

Changelog for the GOV.UK frontend toolkit:
https://github.com/alphagov/govuk_frontend_toolkit/blob/master/CHANGELOG
.md

* Remove DEPRECATED selection-buttons.js

This is no longer required with the radios and checkboxes released in
govuk-elements-sass v3.0.0.

* Update the radio buttons and checkboxes example page

To use the new markup released in GOV.UK elements 3.0.0.

* Replace .block-label with .multiple-choice

Update examples of radio buttons and checkboxes.

Replace ‘.block-label` with `.multiple-choice` as the block-label class
no longer exists.

* Add a .form-section wrapper to examples

Increase the spacing between the radio button and checkbox examples.

* Mention requirement for show-hide-content.js

* Fix radios and checkbox example page

This had nested form elements, which was a mistake. Removing these.

* Add an example of the task list pattern

Add an example of the task list pattern to the docs. Link to it from
the tutorials and examples page.

* Replace em values with gutter variables

Use variables from the govuk_frontend_toolkit’s _measurements.scss file.

Also add a new variable for the spacing to the left of the task list.

* Use the phase tag mixin for the completed status

* Make the "completed" status uppercase

* Rename the task-list to "task-list-page"

* Added config to turn off browser sync

* Add SSH Key for Travis

* Combine scripts and move to a deploy provider

This works in the same way as govuk_elements.

Create a new file - create-release.sh, combining - create release tag
and update release branch, these are deleted.

* Add missing EOF

* To maintain consistency, in this commit I have:
- assigned config.useBrowserSync to a variable
- ran the toLowerCase method on the variable
- change the if statement to reflect the new variable

* add auto data store

* add clear data link to docs app

* only run autodata once, include JS in docs

* use 'data' hash in locals

* use new multiple-choice css, use hyphens in names

* store GET data

* seperate guidance and example pages, added h1s

* refactor checked function

* use list for vehicle features

* fix return to examples link

* refactored docs

* set cookie name and timeout, use crypto

* de-duplicate forceHttps

* Add template pages for content and questions

* Add new template pages to examples index

* v6.0.0

* attempted fix for radio auto stores

* attempted fix for radio auto stores
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants