Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(ngAria): bind to keydown instead of keypress in ngClick #14065

Closed
wants to merge 5 commits into from
Closed

fix(ngAria): bind to keydown instead of keypress in ngClick #14065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3071bb7
fix(ngAria): bind to `keydown` instead of `keypress` in `ngClick`
LeeAdcock Feb 17, 2016
8a5792f
doc(ngAria) fix documentation typo
LeeAdcock Feb 17, 2016
558e9a9
fix(ngAria): bind to `keydown` instead of `keypress` in `ngClick`
LeeAdcock Feb 17, 2016
78f6603
doc(ngAria): update reference to keypress to match new behavior
LeeAdcock Feb 18, 2016
465118a
fix(ngAria): do not bind to `keydown` when any key event handler is a…
LeeAdcock Feb 18, 2016
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
10 changes: 5 additions & 5 deletions src/ngAria/aria.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, keyup event, keypress event, button role |
*
* Find out more information about each directive by reading the
* {@link guide/accessibility ngAria Developer Guide}.
Expand Down Expand Up @@ -96,7 +96,7 @@ function $AriaProvider() {
ariaInvalid: true,
ariaValue: true,
tabindex: true,
bindKeypress: true,
bindKeyEvents: true,
bindRoleForClick: true
};

Expand All @@ -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
* - **bindKeypress** – `{boolean}` – Enables/disables keypress 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
Expand Down Expand Up @@ -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('bindKeyEvents') && !attr.ngKeypress && !attr.ngKeydown && !attr.ngKeyup) {
elem.on('keydown', function(event) {
var keyCode = event.which || event.keyCode;
if (keyCode === 32 || keyCode === 13) {
scope.$apply(callback);
Expand Down
56 changes: 43 additions & 13 deletions test/ngAria/ariaSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -630,14 +630,42 @@ 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() {
it('should not bind to key events if there is existing ng-keydown', function() {
Copy link
Member

Choose a reason for hiding this comment

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

Don't remove this test, just change it to use keydown instead.

scope.someOtherAction = function() {};
var keydownFn = spyOn(scope, 'someOtherAction');

scope.someAction = function() {};
clickFn = spyOn(scope, 'someAction');
compileElement('<div ng-click="someAction()" ng-keydown="someOtherAction()" tabindex="0"></div>');

element.triggerHandler({type: 'keydown', keyCode: 32});

expect(clickFn).not.toHaveBeenCalled();
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('<div ng-click="someAction()" ng-keyup="someOtherAction()" tabindex="0"></div>');

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');

Expand All @@ -651,11 +679,11 @@ describe('$aria', function() {
expect(keypressFn).toHaveBeenCalled();
});

it('should update bindings when keypress handled', function() {
it('should update bindings when keydown handled', function() {
compileElement('<div ng-click="text = \'clicked!\'">{{text}}</div>');
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();
});
Expand All @@ -664,14 +692,14 @@ describe('$aria', function() {
compileElement('<div ng-click="event = $event">{{event.type}}' +
'{{event.keyCode}}</div>');
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('<button ng-click="event = $event">{{event.type}}{{event.keyCode}}</button>');
expect(element.text()).toBe('');
element.triggerHandler({ type: 'keypress', keyCode: 13 });
element.triggerHandler({ type: 'keydown', keyCode: 13 });
expect(element.text()).toBe('');
});
});
Expand All @@ -688,9 +716,9 @@ describe('$aria', function() {
});
});

describe('actions when bindKeypress is set to false', function() {
describe('actions when bindKeyEvents is set to false', function() {
beforeEach(configAriaProvider({
bindKeypress: false
bindKeyEvents: false
}));
beforeEach(injectScopeAndCompiler);

Expand All @@ -700,7 +728,9 @@ describe('$aria', function() {

element = $compile('<div ng-click="someAction()" tabindex="0"></div>')(scope);

element.triggerHandler({type: 'keydown', keyCode: 32});
element.triggerHandler({type: 'keypress', keyCode: 32});
element.triggerHandler({type: 'keyup', keyCode: 32});

expect(clickFn).not.toHaveBeenCalled();
});
Expand Down