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

Commit

Permalink
fix(ngAria): clean up tabindex usage
Browse files Browse the repository at this point in the history 
* Do not put tabindex on native controls using ng-model or ng-click
* Uses a single nodeBlacklist to limit which elements receive support

Closes #11500
  • Loading branch information
@marcysutton @gkalpak
marcysutton authored and gkalpak committed Sep 7, 2015
1 parent ebba426 commit f48244c
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 56 deletions.
74 changes: 39 additions & 35 deletions src/ngAria/aria.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,16 @@
var ngAriaModule = angular.module('ngAria', ['ng']).
provider('$aria', $AriaProvider);

/**
* Internal Utilities
*/
var nodeBlackList = ['BUTTON', 'A', 'INPUT', 'TEXTAREA', 'SELECT', 'DETAILS', 'SUMMARY'];

var isNodeOneOf = function (elem, nodeTypeArray) {
if (nodeTypeArray.indexOf(elem[0].nodeName) !== -1) {
return true;
}
}
/**
* @ngdoc provider
* @name $ariaProvider
Expand Down Expand Up @@ -113,10 +123,10 @@ function $AriaProvider() {
config = angular.extend(config, newConfig);
};

function watchExpr(attrName, ariaAttr, negate) {
function watchExpr(attrName, ariaAttr, nodeBlackList, negate) {
return function(scope, elem, attr) {
var ariaCamelName = attr.$normalize(ariaAttr);
if (config[ariaCamelName] && !attr[ariaCamelName]) {
if (config[ariaCamelName] && !isNodeOneOf(elem, nodeBlackList) && !attr[ariaCamelName]) {
scope.$watch(attr[attrName], function(boolVal) {
// ensure boolean value
boolVal = negate ? !boolVal : !!boolVal;
Expand All @@ -125,7 +135,6 @@ function $AriaProvider() {
}
};
}

/**
* @ngdoc service
* @name $aria
Expand Down Expand Up @@ -184,10 +193,10 @@ function $AriaProvider() {


ngAriaModule.directive('ngShow', ['$aria', function($aria) {
return $aria.$$watchExpr('ngShow', 'aria-hidden', true);
return $aria.$$watchExpr('ngShow', 'aria-hidden', [], true);
}])
.directive('ngHide', ['$aria', function($aria) {
return $aria.$$watchExpr('ngHide', 'aria-hidden', false);
return $aria.$$watchExpr('ngHide', 'aria-hidden', [], false);
}])
.directive('ngModel', ['$aria', function($aria) {

Expand Down Expand Up @@ -261,6 +270,9 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
scope.$watch(ngAriaWatchModelValue, shape === 'radio' ?
getRadioReaction() : ngAriaCheckboxReaction);
}
if (needsTabIndex) {
elem.attr('tabindex', 0);
}
break;
case 'range':
if (shouldAttachRole(shape, elem)) {
Expand Down Expand Up @@ -289,6 +301,9 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
});
}
}
if (needsTabIndex) {
elem.attr('tabindex', 0);
}
break;
case 'multiline':
if (shouldAttachAttr('aria-multiline', 'ariaMultiline', elem)) {
Expand All @@ -297,10 +312,6 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
break;
}

if (needsTabIndex) {
elem.attr('tabindex', 0);
}

if (ngModel.$validators.required && shouldAttachAttr('aria-required', 'ariaRequired', elem)) {
scope.$watch(function ngAriaRequiredWatch() {
return ngModel.$error.required;
Expand All @@ -322,7 +333,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
};
}])
.directive('ngDisabled', ['$aria', function($aria) {
return $aria.$$watchExpr('ngDisabled', 'aria-disabled');
return $aria.$$watchExpr('ngDisabled', 'aria-disabled', []);
}])
.directive('ngMessages', function() {
return {
Expand All @@ -342,43 +353,36 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
var fn = $parse(attr.ngClick, /* interceptorFn */ null, /* expensiveChecks */ true);
return function(scope, elem, attr) {

var nodeBlackList = ['BUTTON', 'A', 'INPUT', 'TEXTAREA'];
if (!isNodeOneOf(elem, nodeBlackList)) {

function isNodeOneOf(elem, nodeTypeArray) {
if (nodeTypeArray.indexOf(elem[0].nodeName) !== -1) {
return true;
if ($aria.config('bindRoleForClick') && !elem.attr('role')) {
elem.attr('role', 'button');
}
}

if ($aria.config('bindRoleForClick')
&& !elem.attr('role')
&& !isNodeOneOf(elem, nodeBlackList)) {
elem.attr('role', 'button');
}

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

if ($aria.config('bindKeypress') && !attr.ngKeypress && !isNodeOneOf(elem, nodeBlackList)) {
elem.on('keypress', function(event) {
var keyCode = event.which || event.keyCode;
if (keyCode === 32 || keyCode === 13) {
scope.$apply(callback);
}
if ($aria.config('bindKeypress') && !attr.ngKeypress) {
elem.on('keypress', function(event) {
var keyCode = event.which || event.keyCode;
if (keyCode === 32 || keyCode === 13) {
scope.$apply(callback);
}

function callback() {
fn(scope, { $event: event });
}
});
function callback() {
fn(scope, { $event: event });
}
});
}
}
};
}
};
}])
.directive('ngDblclick', ['$aria', function($aria) {
return function(scope, elem, attr) {
if ($aria.config('tabindex') && !elem.attr('tabindex')) {
if ($aria.config('tabindex') && !elem.attr('tabindex') && !isNodeOneOf(elem, nodeBlackList)) {
elem.attr('tabindex', 0);
}
};
Expand Down
47 changes: 26 additions & 21 deletions test/ngAria/ariaSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -616,10 +616,35 @@ describe('$aria', function() {
describe('tabindex', function() {
beforeEach(injectScopeAndCompiler);

it('should attach tabindex to role="checkbox", ng-click, and ng-dblclick', function() {
it('should not attach to native controls', function() {
var element = [
$compile("<button ng-click='something'></button>")(scope),
$compile("<a ng-href='#/something'>")(scope),
$compile("<input ng-model='val'>")(scope),
$compile("<textarea ng-model='val'></textarea>")(scope),
$compile("<select ng-model='val'></select>")(scope),
$compile("<details ng-model='val'></details>")(scope)
];
expectAriaAttrOnEachElement(element, 'tabindex', undefined);
});

it('should not attach to random ng-model elements', function() {
compileElement('<div ng-model="val"></div>');
expect(element.attr('tabindex')).toBeUndefined();
});

it('should attach tabindex to custom inputs', function() {
compileElement('<div type="checkbox" ng-model="val"></div>');
expect(element.attr('tabindex')).toBe('0');

compileElement('<div role="checkbox" ng-model="val"></div>');
expect(element.attr('tabindex')).toBe('0');

compileElement('<div type="range" ng-model="val"></div>');
expect(element.attr('tabindex')).toBe('0');
});

it('should attach to ng-click and ng-dblclick', function() {
compileElement('<div ng-click="someAction()"></div>');
expect(element.attr('tabindex')).toBe('0');

Expand All @@ -640,26 +665,6 @@ describe('$aria', function() {
compileElement('<div ng-dblclick="someAction()" tabindex="userSetValue"></div>');
expect(element.attr('tabindex')).toBe('userSetValue');
});

it('should set proper tabindex values for radiogroup', function() {
compileElement('<div role="radiogroup">' +
'<div role="radio" ng-model="val" value="one">1</div>' +
'<div role="radio" ng-model="val" value="two">2</div>' +
'</div>');

var one = element.contents().eq(0);
var two = element.contents().eq(1);

scope.$apply("val = 'one'");
expect(one.attr('tabindex')).toBe('0');
expect(two.attr('tabindex')).toBe('-1');

scope.$apply("val = 'two'");
expect(one.attr('tabindex')).toBe('-1');
expect(two.attr('tabindex')).toBe('0');

dealoc(element);
});
});

describe('accessible actions', function() {
Expand Down

0 comments on commit f48244c

Please sign in to comment.