From c332e047bfaf21087ad12471ca1b595205134001 Mon Sep 17 00:00:00 2001 From: Nick Colley Date: Fri, 28 Oct 2016 10:58:13 +0100 Subject: [PATCH 1/3] Update the shim link handler to use keydown instead of keyup 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 --- javascripts/govuk/shim-links-with-button-role.js | 2 +- spec/unit/shim-links-with-button-role.spec.js | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/javascripts/govuk/shim-links-with-button-role.js b/javascripts/govuk/shim-links-with-button-role.js index c6ced397..833b6b4f 100644 --- a/javascripts/govuk/shim-links-with-button-role.js +++ b/javascripts/govuk/shim-links-with-button-role.js @@ -48,7 +48,7 @@ // 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)) + $(document).on('keydown', this.config.selector, this.triggerClickOnTarget.bind(this)) } } diff --git a/spec/unit/shim-links-with-button-role.spec.js b/spec/unit/shim-links-with-button-role.spec.js index 1e3d27eb..055f005d 100644 --- a/spec/unit/shim-links-with-button-role.spec.js +++ b/spec/unit/shim-links-with-button-role.spec.js @@ -7,7 +7,7 @@ describe('shim-links-with-button-role', function () { var GOVUK = window.GOVUK var $buttonLink - var keyupEvent + var keyDownEvent beforeEach(function () { $buttonLink = $('Button') @@ -15,8 +15,8 @@ describe('shim-links-with-button-role', 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() }) @@ -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) }) }) From 196c7d4b22021ea01ef41edba790530a59a42442 Mon Sep 17 00:00:00 2001 From: Nick Colley Date: Fri, 28 Oct 2016 11:00:04 +0100 Subject: [PATCH 2/3] Simplify shim links shim 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" --- .../govuk/shim-links-with-button-role.js | 54 +++++-------------- 1 file changed, 14 insertions(+), 40 deletions(-) diff --git a/javascripts/govuk/shim-links-with-button-role.js b/javascripts/govuk/shim-links-with-button-role.js index 833b6b4f..f572a6ee 100644 --- a/javascripts/govuk/shim-links-with-button-role.js +++ b/javascripts/govuk/shim-links-with-button-role.js @@ -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) +// Will tell a user to press space on a 'button', so this functionality needs to be shimmed // -// 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' @@ -16,40 +14,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('keydown', 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) { + event.preventDefault() + // trigger the target's click event + event.target.click() + } + }) } } From 472fabcd1bfcee50c00b27e943f8a1b72f86657c Mon Sep 17 00:00:00 2001 From: Nick Colley Date: Mon, 7 Nov 2016 11:49:37 +0000 Subject: [PATCH 3/3] Add link for context of why we're shimming this --- javascripts/govuk/shim-links-with-button-role.js | 1 + 1 file changed, 1 insertion(+) diff --git a/javascripts/govuk/shim-links-with-button-role.js b/javascripts/govuk/shim-links-with-button-role.js index f572a6ee..e9f1dc7a 100644 --- a/javascripts/govuk/shim-links-with-button-role.js +++ b/javascripts/govuk/shim-links-with-button-role.js @@ -3,6 +3,7 @@ // // Created since some Assistive Technologies (for example some Screenreaders) // 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 // // Usage instructions: // GOVUK.shimLinksWithButtonRole.init();