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

Commit

Permalink
bug(ngAria): native controls fire a single click
Browse files Browse the repository at this point in the history
Closes #10388
  • Loading branch information
Marcy Sutton committed Feb 6, 2015
1 parent e24d968 commit 0e4f397
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
13 changes: 8 additions & 5 deletions docs/content/guide/accessibility.ngdoc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Currently, ngAria interfaces with the following directives:
* {@link guide/accessibility#nghide ngHide}
* {@link guide/accessibility#ngclick ngClick}
* {@link guide/accessibility#ngdblclick ngDblClick}
* {@link guide/accessibility#ngmessages ngMessages}

<h2 id="ngmodel">ngModel</h2>

Expand Down Expand Up @@ -209,10 +210,13 @@ The default CSS for `ngHide`, the inverse method to `ngShow`, makes ngAria redun
`display: none`. See explanation for {@link guide/accessibility#ngshow ngShow} when overriding the default CSS.

<h2><span id="ngclick">ngClick</span> and <span id="ngdblclick">ngDblclick</span></h2>
If `ng-click` or `ng-dblclick` is encountered, ngAria will add `tabindex` if it isn't there already.
Even with this, you must currently still add `ng-keypress` to non-interactive elements such as `div`
or `taco-button` to enable keyboard access. Conversation is currently ongoing about whether ngAria
should also bind `ng-keypress`.
If `ng-click` or `ng-dblclick` is encountered, ngAria will add `tabindex="0"` if it isn't there
already.

For `ng-click`, keypress will also be bound to `div` and `li` elements. You can turn this functionality on or off with the `bindKeypress` configuration option.

For `ng-dblclick`, you must manually add `ng-keypress` to non-interactive elements such as `div`
or `taco-button` to enable keyboard access.

<h3>Example</h3>
```html
Expand All @@ -223,7 +227,6 @@ Becomes:
```html
<div ng-click="toggleMenu()" tabindex="0"></div>
```
*Note: ngAria still requires `ng-keypress` to be added manually to non-native controls like divs.*

<h2 id="ngmessages">ngMessages</h2>

Expand Down
12 changes: 10 additions & 2 deletions src/ngAria/aria.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ function $AriaProvider() {
* - **ariaMultiline** – `{boolean}` – Enables/disables aria-multiline 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 ng-click
* - **bindKeypress** – `{boolean}` – Enables/disables keypress event binding on `&lt;div&gt;` and
* `&lt;li&gt;` elements with ng-click
*
* @description
* Enables/disables various ARIA attributes
Expand Down Expand Up @@ -303,11 +304,18 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
compile: function(elem, attr) {
var fn = $parse(attr.ngClick, /* interceptorFn */ null, /* expensiveChecks */ true);
return function(scope, elem, attr) {

function isNodeOneOf(elem, nodeTypeArray) {
if (nodeTypeArray.indexOf(elem[0].nodeName) !== -1) {
return true;
}
}

if ($aria.config('tabindex') && !elem.attr('tabindex')) {
elem.attr('tabindex', 0);
}

if ($aria.config('bindKeypress') && !attr.ngKeypress) {
if ($aria.config('bindKeypress') && !attr.ngKeypress && isNodeOneOf(elem, ['DIV', 'LI'])) {
elem.on('keypress', function(event) {
if (event.keyCode === 32 || event.keyCode === 13) {
scope.$apply(callback);
Expand Down
31 changes: 25 additions & 6 deletions test/ngAria/ariaSpec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

describe('$aria', function() {
ddescribe('$aria', function() {
var scope, $compile, element;

beforeEach(module('ngAria'));
Expand Down Expand Up @@ -488,12 +488,24 @@ describe('$aria', function() {

it('should a trigger click from the keyboard', function() {
scope.someAction = function() {};
compileInput('<div ng-click="someAction()" tabindex="0"></div>');

var elements = $compile('<section>' +
'<div class="div-click" ng-click="someAction(\'div\')" tabindex="0"></div>' +
'<ul><li ng-click="someAction( \'li\')" tabindex="0"></li></ul>' +
'</section>')(scope);

scope.$digest();

clickFn = spyOn(scope, 'someAction');

element.triggerHandler({type: 'keypress', keyCode: 32});
var divElement = elements.find('div');
var liElement = elements.find('li');

divElement.triggerHandler({type: 'keypress', keyCode: 32});
liElement.triggerHandler({type: 'keypress', keyCode: 32});

expect(clickFn).toHaveBeenCalled();
expect(clickFn).toHaveBeenCalledWith('div');
expect(clickFn).toHaveBeenCalledWith('li');
});

it('should not override existing ng-keypress', function() {
Expand Down Expand Up @@ -526,6 +538,13 @@ describe('$aria', function() {
element.triggerHandler({ type: 'keypress', keyCode: 13 });
expect(element.text()).toBe('keypress13');
});

it('should not bind keypress to elements not in the default config', function() {
compileInput('<button ng-click="event = $event">{{event.type}}{{event.keyCode}}</button>');
expect(element.text()).toBe('');
element.triggerHandler({ type: 'keypress', keyCode: 13 });
expect(element.text()).toBe('');
});
});

describe('actions when bindKeypress set to false', function() {
Expand All @@ -534,11 +553,11 @@ describe('$aria', function() {
}));
beforeEach(injectScopeAndCompiler);

it('should not a trigger click from the keyboard', function() {
it('should not a trigger click', function() {
scope.someAction = function() {};
var clickFn = spyOn(scope, 'someAction');

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

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

Expand Down

0 comments on commit 0e4f397

Please sign in to comment.