Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c37c8a8

Browse files
marcysuttonmarcysutton
committedJul 2, 2015
fix(ngAria): clean up tabindex usage
* 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
1 parent 1f5e42e commit c37c8a8

File tree

2 files changed

+68
-57
lines changed

2 files changed

+68
-57
lines changed
 

‎src/ngAria/aria.js

+42-36
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ function $AriaProvider() {
8787
bindRoleForClick: true
8888
};
8989

90+
var nodeBlacklist = ['BUTTON', 'A', 'INPUT', 'TEXTAREA', 'SELECT', 'DETAILS', 'SUMMARY'];
91+
9092
/**
9193
* @ngdoc method
9294
* @name $ariaProvider#config
@@ -113,10 +115,10 @@ function $AriaProvider() {
113115
config = angular.extend(config, newConfig);
114116
};
115117

116-
function watchExpr(attrName, ariaAttr, negate) {
118+
function watchExpr(attrName, ariaAttr, nodeBlackList, negate) {
117119
return function(scope, elem, attr) {
118120
var ariaCamelName = attr.$normalize(ariaAttr);
119-
if (config[ariaCamelName] && !attr[ariaCamelName]) {
121+
if (config[ariaCamelName] && !isNodeOneOf(elem, nodeBlackList) && !attr[ariaCamelName]) {
120122
scope.$watch(attr[attrName], function(boolVal) {
121123
// ensure boolean value
122124
boolVal = negate ? !boolVal : !!boolVal;
@@ -126,6 +128,11 @@ function $AriaProvider() {
126128
};
127129
}
128130

131+
function isNodeOneOf(elem, nodeTypeArray) {
132+
if (nodeTypeArray.indexOf(elem[0].nodeName) !== -1) {
133+
return true;
134+
}
135+
}
129136
/**
130137
* @ngdoc service
131138
* @name $aria
@@ -177,17 +184,19 @@ function $AriaProvider() {
177184
config: function(key) {
178185
return config[key];
179186
},
180-
$$watchExpr: watchExpr
187+
$$watchExpr: watchExpr,
188+
nodeBlackList: nodeBlacklist,
189+
isNodeOneOf: isNodeOneOf
181190
};
182191
};
183192
}
184193

185194

186195
ngAriaModule.directive('ngShow', ['$aria', function($aria) {
187-
return $aria.$$watchExpr('ngShow', 'aria-hidden', true);
196+
return $aria.$$watchExpr('ngShow', 'aria-hidden', [], true);
188197
}])
189198
.directive('ngHide', ['$aria', function($aria) {
190-
return $aria.$$watchExpr('ngHide', 'aria-hidden', false);
199+
return $aria.$$watchExpr('ngHide', 'aria-hidden', [], false);
191200
}])
192201
.directive('ngModel', ['$aria', function($aria) {
193202

@@ -226,7 +235,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
226235
}
227236
},
228237
post: function(scope, elem, attr, ngModel) {
229-
var needsTabIndex = shouldAttachAttr('tabindex', 'tabindex', elem);
238+
var needsTabIndex = shouldAttachAttr('tabindex', 'tabindex', elem, $aria.nodeBlackList);
230239

231240
function ngAriaWatchModelValue() {
232241
return ngModel.$modelValue;
@@ -261,6 +270,9 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
261270
scope.$watch(ngAriaWatchModelValue, shape === 'radio' ?
262271
getRadioReaction() : ngAriaCheckboxReaction);
263272
}
273+
if (needsTabIndex) {
274+
elem.attr('tabindex', 0);
275+
}
264276
break;
265277
case 'range':
266278
if (shouldAttachRole(shape, elem)) {
@@ -289,6 +301,9 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
289301
});
290302
}
291303
}
304+
if (needsTabIndex) {
305+
elem.attr('tabindex', 0);
306+
}
292307
break;
293308
case 'multiline':
294309
if (shouldAttachAttr('aria-multiline', 'ariaMultiline', elem)) {
@@ -297,10 +312,6 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
297312
break;
298313
}
299314

300-
if (needsTabIndex) {
301-
elem.attr('tabindex', 0);
302-
}
303-
304315
if (ngModel.$validators.required && shouldAttachAttr('aria-required', 'ariaRequired', elem)) {
305316
scope.$watch(function ngAriaRequiredWatch() {
306317
return ngModel.$error.required;
@@ -322,7 +333,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
322333
};
323334
}])
324335
.directive('ngDisabled', ['$aria', function($aria) {
325-
return $aria.$$watchExpr('ngDisabled', 'aria-disabled');
336+
return $aria.$$watchExpr('ngDisabled', 'aria-disabled', []);
326337
}])
327338
.directive('ngMessages', function() {
328339
return {
@@ -342,43 +353,38 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) {
342353
var fn = $parse(attr.ngClick, /* interceptorFn */ null, /* expensiveChecks */ true);
343354
return function(scope, elem, attr) {
344355

345-
var nodeBlackList = ['BUTTON', 'A', 'INPUT', 'TEXTAREA'];
356+
if (!$aria.isNodeOneOf(elem, $aria.nodeBlackList)) {
346357

347-
function isNodeOneOf(elem, nodeTypeArray) {
348-
if (nodeTypeArray.indexOf(elem[0].nodeName) !== -1) {
349-
return true;
358+
if ($aria.config('bindRoleForClick') && !elem.attr('role')) {
359+
elem.attr('role', 'button');
350360
}
351-
}
352-
353-
if ($aria.config('bindRoleForClick')
354-
&& !elem.attr('role')
355-
&& !isNodeOneOf(elem, nodeBlackList)) {
356-
elem.attr('role', 'button');
357-
}
358361

359-
if ($aria.config('tabindex') && !elem.attr('tabindex')) {
360-
elem.attr('tabindex', 0);
361-
}
362+
if ($aria.config('tabindex') && !elem.attr('tabindex')) {
363+
elem.attr('tabindex', 0);
364+
}
362365

363-
if ($aria.config('bindKeypress') && !attr.ngKeypress && !isNodeOneOf(elem, nodeBlackList)) {
364-
elem.on('keypress', function(event) {
365-
var keyCode = event.which || event.keyCode;
366-
if (keyCode === 32 || keyCode === 13) {
367-
scope.$apply(callback);
368-
}
366+
if ($aria.config('bindKeypress') && !attr.ngKeypress) {
367+
elem.on('keypress', function(event) {
368+
var keyCode = event.which || event.keyCode;
369+
if (keyCode === 32 || keyCode === 13) {
370+
scope.$apply(callback);
371+
}
369372

370-
function callback() {
371-
fn(scope, { $event: event });
372-
}
373-
});
373+
function callback() {
374+
fn(scope, { $event: event });
375+
}
376+
});
377+
}
374378
}
375379
};
376380
}
377381
};
378382
}])
379383
.directive('ngDblclick', ['$aria', function($aria) {
380384
return function(scope, elem, attr) {
381-
if ($aria.config('tabindex') && !elem.attr('tabindex')) {
385+
if ($aria.config('tabindex')
386+
&& !elem.attr('tabindex')
387+
&& !$aria.isNodeOneOf(elem, $aria.nodeBlackList)) {
382388
elem.attr('tabindex', 0);
383389
}
384390
};

‎test/ngAria/ariaSpec.js

+26-21
Original file line numberDiff line numberDiff line change
@@ -616,10 +616,35 @@ describe('$aria', function() {
616616
describe('tabindex', function() {
617617
beforeEach(injectScopeAndCompiler);
618618

619-
it('should attach tabindex to role="checkbox", ng-click, and ng-dblclick', function() {
619+
it('should not attach to native controls', function() {
620+
var element = [
621+
$compile("<button ng-click='something'></button>")(scope),
622+
$compile("<a ng-href='#/something'>")(scope),
623+
$compile("<input ng-model='val'>")(scope),
624+
$compile("<textarea ng-model='val'></textarea>")(scope),
625+
$compile("<select ng-model='val'></select>")(scope),
626+
$compile("<details ng-model='val'></details>")(scope)
627+
];
628+
expectAriaAttrOnEachElement(element, 'tabindex', undefined);
629+
});
630+
631+
it('should not attach to rando ng-model elements', function() {
632+
compileElement('<div ng-model="val"></div>');
633+
expect(element.attr('tabindex')).toBeUndefined();
634+
});
635+
636+
it('should attach tabindex to custom inputs', function() {
637+
compileElement('<div type="checkbox" ng-model="val"></div>');
638+
expect(element.attr('tabindex')).toBe('0');
639+
620640
compileElement('<div role="checkbox" ng-model="val"></div>');
621641
expect(element.attr('tabindex')).toBe('0');
622642

643+
compileElement('<div type="range" ng-model="val"></div>');
644+
expect(element.attr('tabindex')).toBe('0');
645+
});
646+
647+
it('should attach to ng-click and ng-dblclick', function() {
623648
compileElement('<div ng-click="someAction()"></div>');
624649
expect(element.attr('tabindex')).toBe('0');
625650

@@ -640,26 +665,6 @@ describe('$aria', function() {
640665
compileElement('<div ng-dblclick="someAction()" tabindex="userSetValue"></div>');
641666
expect(element.attr('tabindex')).toBe('userSetValue');
642667
});
643-
644-
it('should set proper tabindex values for radiogroup', function() {
645-
compileElement('<div role="radiogroup">' +
646-
'<div role="radio" ng-model="val" value="one">1</div>' +
647-
'<div role="radio" ng-model="val" value="two">2</div>' +
648-
'</div>');
649-
650-
var one = element.contents().eq(0);
651-
var two = element.contents().eq(1);
652-
653-
scope.$apply("val = 'one'");
654-
expect(one.attr('tabindex')).toBe('0');
655-
expect(two.attr('tabindex')).toBe('-1');
656-
657-
scope.$apply("val = 'two'");
658-
expect(one.attr('tabindex')).toBe('-1');
659-
expect(two.attr('tabindex')).toBe('0');
660-
661-
dealoc(element);
662-
});
663668
});
664669

665670
describe('accessible actions', function() {

0 commit comments

Comments
 (0)
This repository has been archived.