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

bug(ngAria): native controls fire a single click event #10766

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 9 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,14 @@ 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 +228,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
29 changes: 24 additions & 5 deletions test/ngAria/ariaSpec.js
Original file line number Diff line number Diff line change
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