Skip to content

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 angular#11500
  • Loading branch information
@marcysutton
marcysutton committed Jul 2, 2015
1 parent 1f5e42e commit c37c8a8
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 57 deletions.
78 changes: 42 additions & 36 deletions src/ngAria/aria.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ function $AriaProvider() {
bindRoleForClick: true
};

var nodeBlacklist = ['BUTTON', 'A', 'INPUT', 'TEXTAREA', 'SELECT', 'DETAILS', 'SUMMARY'];

/**
* @ngdoc method
* @name $ariaProvider#config
Expand All @@ -113,10 +115,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 @@ -126,6 +128,11 @@ function $AriaProvider() {
};
}

function isNodeOneOf(elem, nodeTypeArray) {
if (nodeTypeArray.indexOf(elem[0].nodeName) !== -1) {
return true;
}
}
/**
* @ngdoc service
* @name $aria
Expand Down Expand Up @@ -177,17 +184,19 @@ function $AriaProvider() {
config: function(key) {
return config[key];
},
$$watchExpr: watchExpr
$$watchExpr: watchExpr,
nodeBlackList: nodeBlacklist,
isNodeOneOf: isNodeOneOf
};
};
}


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 @@ -226,7 +235,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
}
},
post: function(scope, elem, attr, ngModel) {
var needsTabIndex = shouldAttachAttr('tabindex', 'tabindex', elem);
var needsTabIndex = shouldAttachAttr('tabindex', 'tabindex', elem, $aria.nodeBlackList);

function ngAriaWatchModelValue() {
return ngModel.$modelValue;
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,38 @@ 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 (!$aria.isNodeOneOf(elem, $aria.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')
&& !$aria.isNodeOneOf(elem, $aria.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 rando 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 c37c8a8

Please sign in to comment.