From 3071bb7488c0461b7c61af62acc4a98df152db3a Mon Sep 17 00:00:00 2001 From: Lee Adcock Date: Wed, 17 Feb 2016 03:57:58 +0000 Subject: [PATCH 1/5] fix(ngAria): bind to `keydown` instead of `keypress` in `ngClick` The ngAria keyboard support for html elements used as button now binds to the keydown event instead of keypress to better emulate behavior of native buttons. BREAKING CHANGE: The ngAria configuration flag that controls binding to keyboard input has been renamed from `bindKeypress` to `bindKeydown` so that it accurately dscribes the implemented behavior. Closes #14063 --- src/ngAria/aria.js | 10 +++++----- test/ngAria/ariaSpec.js | 40 +++++++++++++--------------------------- 2 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index 400d951e2bc0..e9647b72af56 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -96,7 +96,7 @@ function $AriaProvider() { ariaInvalid: true, ariaValue: true, tabindex: true, - bindKeypress: true, + bindKeydown: true, bindRoleForClick: true }; @@ -113,8 +113,8 @@ function $AriaProvider() { * - **ariaInvalid** – `{boolean}` – Enables/disables aria-invalid tags * - **ariaValue** – `{boolean}` – Enables/disables aria-valuemin, aria-valuemax and aria-valuenow tags * - **tabindex** – `{boolean}` – Enables/disables tabindex tags - * - **bindKeypress** – `{boolean}` – Enables/disables keypress event binding on `div` and - * `li` elements with ng-click + * - **bindKeydown** – `{boolean}` – Enables/disables keydown event binding on `div` and + * `li;` elements with ng-click * - **bindRoleForClick** – `{boolean}` – Adds role=button to non-interactive elements like `div` * using ng-click, making them more accessible to users of assistive technologies * @@ -364,8 +364,8 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { elem.attr('tabindex', 0); } - if ($aria.config('bindKeypress') && !attr.ngKeypress) { - elem.on('keypress', function(event) { + if ($aria.config('bindKeydown') && !attr.ngKeypress) { + elem.on('keydown', function(event) { var keyCode = event.which || event.keyCode; if (keyCode === 32 || keyCode === 13) { scope.$apply(callback); diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index 37c4f17bf1d8..da333f170548 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -608,8 +608,8 @@ describe('$aria', function() { var divElement = elements.find('div'); var liElement = elements.find('li'); - divElement.triggerHandler({type: 'keypress', keyCode: 32}); - liElement.triggerHandler({type: 'keypress', keyCode: 32}); + divElement.triggerHandler({type: 'keydown', keyCode: 32}); + liElement.triggerHandler({type: 'keydown', keyCode: 32}); expect(clickFn).toHaveBeenCalledWith('div'); expect(clickFn).toHaveBeenCalledWith('li'); @@ -630,32 +630,18 @@ describe('$aria', function() { var divElement = elements.find('div'); var liElement = elements.find('li'); - divElement.triggerHandler({type: 'keypress', which: 32}); - liElement.triggerHandler({type: 'keypress', which: 32}); + divElement.triggerHandler({type: 'keydown', which: 32}); + liElement.triggerHandler({type: 'keydown', which: 32}); expect(clickFn).toHaveBeenCalledWith('div'); expect(clickFn).toHaveBeenCalledWith('li'); }); - it('should not override existing ng-keypress', function() { - scope.someOtherAction = function() {}; - var keypressFn = spyOn(scope, 'someOtherAction'); - - scope.someAction = function() {}; - clickFn = spyOn(scope, 'someAction'); - compileElement('
'); - - element.triggerHandler({type: 'keypress', keyCode: 32}); - - expect(clickFn).not.toHaveBeenCalled(); - expect(keypressFn).toHaveBeenCalled(); - }); - - it('should update bindings when keypress handled', function() { + it('should update bindings when keydown handled', function() { compileElement('
{{text}}
'); expect(element.text()).toBe(''); spyOn(scope.$root, '$digest').andCallThrough(); - element.triggerHandler({ type: 'keypress', keyCode: 13 }); + element.triggerHandler({ type: 'keydown', keyCode: 13 }); expect(element.text()).toBe('clicked!'); expect(scope.$root.$digest).toHaveBeenCalledOnce(); }); @@ -664,14 +650,14 @@ describe('$aria', function() { compileElement('
{{event.type}}' + '{{event.keyCode}}
'); expect(element.text()).toBe(''); - element.triggerHandler({ type: 'keypress', keyCode: 13 }); - expect(element.text()).toBe('keypress13'); + element.triggerHandler({ type: 'keydown', keyCode: 13 }); + expect(element.text()).toBe('keydown13'); }); - it('should not bind keypress to elements not in the default config', function() { + it('should not bind keydown to elements not in the default config', function() { compileElement(''); expect(element.text()).toBe(''); - element.triggerHandler({ type: 'keypress', keyCode: 13 }); + element.triggerHandler({ type: 'keydown', keyCode: 13 }); expect(element.text()).toBe(''); }); }); @@ -688,9 +674,9 @@ describe('$aria', function() { }); }); - describe('actions when bindKeypress is set to false', function() { + describe('actions when bindKeydown is set to false', function() { beforeEach(configAriaProvider({ - bindKeypress: false + bindKeydown: false })); beforeEach(injectScopeAndCompiler); @@ -700,7 +686,7 @@ describe('$aria', function() { element = $compile('
')(scope); - element.triggerHandler({type: 'keypress', keyCode: 32}); + element.triggerHandler({type: 'keydown', keyCode: 32}); expect(clickFn).not.toHaveBeenCalled(); }); From 8a5792fe12d5ea3654ab6453f5c684338ae133e1 Mon Sep 17 00:00:00 2001 From: Lee Adcock Date: Wed, 17 Feb 2016 04:33:29 +0000 Subject: [PATCH 2/5] doc(ngAria) fix documentation typo --- src/ngAria/aria.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index e9647b72af56..6083510242e6 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -114,7 +114,7 @@ function $AriaProvider() { * - **ariaValue** – `{boolean}` – Enables/disables aria-valuemin, aria-valuemax and aria-valuenow tags * - **tabindex** – `{boolean}` – Enables/disables tabindex tags * - **bindKeydown** – `{boolean}` – Enables/disables keydown event binding on `div` and - * `li;` elements with ng-click + * `li` elements with ng-click * - **bindRoleForClick** – `{boolean}` – Adds role=button to non-interactive elements like `div` * using ng-click, making them more accessible to users of assistive technologies * From 558e9a9a60bbf73b6b2094c908fae4360055d015 Mon Sep 17 00:00:00 2001 From: Lee Adcock Date: Wed, 17 Feb 2016 15:11:48 +0000 Subject: [PATCH 3/5] fix(ngAria): bind to `keydown` instead of `keypress` in `ngClick` --- src/ngAria/aria.js | 2 +- test/ngAria/ariaSpec.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index 6083510242e6..0274c27b4a05 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -364,7 +364,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { elem.attr('tabindex', 0); } - if ($aria.config('bindKeydown') && !attr.ngKeypress) { + if ($aria.config('bindKeydown') && !attr.ngKeydown) { elem.on('keydown', function(event) { var keyCode = event.which || event.keyCode; if (keyCode === 32 || keyCode === 13) { diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index da333f170548..6db7abf4d2c6 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -637,6 +637,20 @@ describe('$aria', function() { expect(clickFn).toHaveBeenCalledWith('li'); }); + it('should not override existing ng-keydown', function() { + scope.someOtherAction = function() {}; + var keydownFn = spyOn(scope, 'someOtherAction'); + + scope.someAction = function() {}; + clickFn = spyOn(scope, 'someAction'); + compileElement('
'); + + element.triggerHandler({type: 'keydown', keyCode: 32}); + + expect(clickFn).not.toHaveBeenCalled(); + expect(keydownFn).toHaveBeenCalled(); + }); + it('should update bindings when keydown handled', function() { compileElement('
{{text}}
'); expect(element.text()).toBe(''); From 78f66034f8a333b8f7c0b48e3e73d9f06a0a1e7f Mon Sep 17 00:00:00 2001 From: Lee Adcock Date: Thu, 18 Feb 2016 01:48:29 +0000 Subject: [PATCH 4/5] doc(ngAria): update reference to keypress to match new behavior --- src/ngAria/aria.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index 0274c27b4a05..f6f23dc0a628 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -32,7 +32,7 @@ * | {@link ng.directive:ngHide ngHide} | aria-hidden | * | {@link ng.directive:ngDblclick ngDblclick} | tabindex | * | {@link module:ngMessages ngMessages} | aria-live | - * | {@link ng.directive:ngClick ngClick} | tabindex, keypress event, button role | + * | {@link ng.directive:ngClick ngClick} | tabindex, keydown event, button role | * * Find out more information about each directive by reading the * {@link guide/accessibility ngAria Developer Guide}. @@ -364,7 +364,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { elem.attr('tabindex', 0); } - if ($aria.config('bindKeydown') && !attr.ngKeydown) { + if ($aria.config('bindKeydown') && !attr.ngKeypress) { elem.on('keydown', function(event) { var keyCode = event.which || event.keyCode; if (keyCode === 32 || keyCode === 13) { From 465118af71a9406ec5224e781e8992a4d466cf56 Mon Sep 17 00:00:00 2001 From: Lee Adcock Date: Thu, 18 Feb 2016 03:05:13 +0000 Subject: [PATCH 5/5] fix(ngAria): do not bind to `keydown` when any key event handler is already defined --- src/ngAria/aria.js | 8 ++++---- test/ngAria/ariaSpec.js | 36 +++++++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index f6f23dc0a628..27755ae64e44 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -32,7 +32,7 @@ * | {@link ng.directive:ngHide ngHide} | aria-hidden | * | {@link ng.directive:ngDblclick ngDblclick} | tabindex | * | {@link module:ngMessages ngMessages} | aria-live | - * | {@link ng.directive:ngClick ngClick} | tabindex, keydown event, button role | + * | {@link ng.directive:ngClick ngClick} | tabindex, keydown event, keyup event, keypress event, button role | * * Find out more information about each directive by reading the * {@link guide/accessibility ngAria Developer Guide}. @@ -96,7 +96,7 @@ function $AriaProvider() { ariaInvalid: true, ariaValue: true, tabindex: true, - bindKeydown: true, + bindKeyEvents: true, bindRoleForClick: true }; @@ -113,7 +113,7 @@ function $AriaProvider() { * - **ariaInvalid** – `{boolean}` – Enables/disables aria-invalid tags * - **ariaValue** – `{boolean}` – Enables/disables aria-valuemin, aria-valuemax and aria-valuenow tags * - **tabindex** – `{boolean}` – Enables/disables tabindex tags - * - **bindKeydown** – `{boolean}` – Enables/disables keydown event binding on `div` and + * - **bindKeyEvents** – `{boolean}` – Enables/disables keyboard event binding on `div` and * `li` elements with ng-click * - **bindRoleForClick** – `{boolean}` – Adds role=button to non-interactive elements like `div` * using ng-click, making them more accessible to users of assistive technologies @@ -364,7 +364,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { elem.attr('tabindex', 0); } - if ($aria.config('bindKeydown') && !attr.ngKeypress) { + if ($aria.config('bindKeyEvents') && !attr.ngKeypress && !attr.ngKeydown && !attr.ngKeyup) { elem.on('keydown', function(event) { var keyCode = event.which || event.keyCode; if (keyCode === 32 || keyCode === 13) { diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index 6db7abf4d2c6..cee944137798 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -637,7 +637,7 @@ describe('$aria', function() { expect(clickFn).toHaveBeenCalledWith('li'); }); - it('should not override existing ng-keydown', function() { + it('should not bind to key events if there is existing ng-keydown', function() { scope.someOtherAction = function() {}; var keydownFn = spyOn(scope, 'someOtherAction'); @@ -651,6 +651,34 @@ describe('$aria', function() { expect(keydownFn).toHaveBeenCalled(); }); + it('should not bind to key events if there is existing ng-keyup', function() { + scope.someOtherAction = function() {}; + var keyupFn = spyOn(scope, 'someOtherAction'); + + scope.someAction = function() {}; + clickFn = spyOn(scope, 'someAction'); + compileElement('
'); + + element.triggerHandler({type: 'keyup', keyCode: 32}); + + expect(clickFn).not.toHaveBeenCalled(); + expect(keyupFn).toHaveBeenCalled(); + }); + + it('should not bind to key events if there is existing ng-keypress', function() { + scope.someOtherAction = function() {}; + var keypressFn = spyOn(scope, 'someOtherAction'); + + scope.someAction = function() {}; + clickFn = spyOn(scope, 'someAction'); + compileElement('
'); + + element.triggerHandler({type: 'keypress', keyCode: 32}); + + expect(clickFn).not.toHaveBeenCalled(); + expect(keypressFn).toHaveBeenCalled(); + }); + it('should update bindings when keydown handled', function() { compileElement('
{{text}}
'); expect(element.text()).toBe(''); @@ -688,9 +716,9 @@ describe('$aria', function() { }); }); - describe('actions when bindKeydown is set to false', function() { + describe('actions when bindKeyEvents is set to false', function() { beforeEach(configAriaProvider({ - bindKeydown: false + bindKeyEvents: false })); beforeEach(injectScopeAndCompiler); @@ -701,6 +729,8 @@ describe('$aria', function() { element = $compile('
')(scope); element.triggerHandler({type: 'keydown', keyCode: 32}); + element.triggerHandler({type: 'keypress', keyCode: 32}); + element.triggerHandler({type: 'keyup', keyCode: 32}); expect(clickFn).not.toHaveBeenCalled(); });