From f129167895a3b59794e410f4d1adf8b03ef0313c Mon Sep 17 00:00:00 2001 From: marcysutton Date: Thu, 2 Jul 2015 10:15:34 -0700 Subject: [PATCH 1/2] 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 --- src/ngAria/aria.js | 74 ++++++++++++++++++++++------------------- test/ngAria/ariaSpec.js | 47 ++++++++++++++------------ 2 files changed, 65 insertions(+), 56 deletions(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index c1038f2f26c7..e8c3bcbff6b2 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -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 @@ -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; @@ -125,7 +135,6 @@ function $AriaProvider() { } }; } - /** * @ngdoc service * @name $aria @@ -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) { @@ -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)) { @@ -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)) { @@ -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; @@ -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 { @@ -342,35 +353,28 @@ 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 }); + } + }); + } } }; } @@ -378,7 +382,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { }]) .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); } }; diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index 864f856ed271..911e6aab73d5 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -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("")(scope), + $compile("")(scope), + $compile("")(scope), + $compile("")(scope), + $compile("")(scope), + $compile("
")(scope) + ]; + expectAriaAttrOnEachElement(element, 'tabindex', undefined); + }); + + it('should not attach to random ng-model elements', function() { + compileElement('
'); + expect(element.attr('tabindex')).toBeUndefined(); + }); + + it('should attach tabindex to custom inputs', function() { + compileElement('
'); + expect(element.attr('tabindex')).toBe('0'); + compileElement('
'); expect(element.attr('tabindex')).toBe('0'); + compileElement('
'); + expect(element.attr('tabindex')).toBe('0'); + }); + + it('should attach to ng-click and ng-dblclick', function() { compileElement('
'); expect(element.attr('tabindex')).toBe('0'); @@ -640,26 +665,6 @@ describe('$aria', function() { compileElement('
'); expect(element.attr('tabindex')).toBe('userSetValue'); }); - - it('should set proper tabindex values for radiogroup', function() { - compileElement('
' + - '
1
' + - '
2
' + - '
'); - - 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() { From fd15b735d948d7fe1e0165ad7b6c0197e65e9e1b Mon Sep 17 00:00:00 2001 From: Marcy Sutton Date: Tue, 4 Aug 2015 11:27:06 -0700 Subject: [PATCH 2/2] doc(ngAria): Update accessibility guide --- docs/content/guide/accessibility.ngdoc | 97 ++++++++------------------ 1 file changed, 30 insertions(+), 67 deletions(-) diff --git a/docs/content/guide/accessibility.ngdoc b/docs/content/guide/accessibility.ngdoc index b37c3e1caf25..76f6fcfc498a 100644 --- a/docs/content/guide/accessibility.ngdoc +++ b/docs/content/guide/accessibility.ngdoc @@ -23,9 +23,9 @@ angular.module('myApp', ['ngAria'])... ###Using ngAria Most of what ngAria does is only visible "under the hood". To see the module in action, once you've added it as a dependency, you can test a few things: - * Using your favorite element inspector, look for ngAria attributes in your own code. + * Using your favorite element inspector, look for attributes added by ngAria in your own code. * Test using your keyboard to ensure `tabindex` is used correctly. - * Fire up a screen reader such as VoiceOver to listen for ARIA support. + * Fire up a screen reader such as VoiceOver or NVDA to check for ARIA support. [Helpful screen reader tips.](http://webaim.org/articles/screenreader_testing/) ##Supported directives @@ -41,8 +41,8 @@ Currently, ngAria interfaces with the following directives:

ngModel

-Most of ngAria's heavy lifting happens in the {@link ngModel ngModel} -directive. For elements using ngModel, special attention is paid by ngAria if that element also +Much of ngAria's heavy lifting happens in the {@link ngModel ngModel} +directive. For elements using ngModel, special attention is paid by ngAria if that element also has a role or type of `checkbox`, `radio`, `range` or `textbox`. For those elements using ngModel, ngAria will dynamically bind and update the following ARIA @@ -134,10 +134,8 @@ attributes (if they have not been explicitly specified by the developer): ngAria will also add `tabIndex`, ensuring custom elements with these roles will be reachable from the keyboard. It is still up to **you** as a developer to **ensure custom controls will be -operable** from the keybard. Think of `ng-click` on a `
` or ``: you still need -to bind `ng-keypress` to make it fully operable from the keyboard. As a rule, any time you create -a widget involving user interaction, be sure to test it with your keyboard and at least one mobile -and desktop screen reader (preferably more). +accessible**. As a rule, any time you create a widget involving user interaction, be sure to test +it with your keyboard and at least one mobile and desktop screen reader.

ngDisabled

@@ -160,7 +158,7 @@ Becomes: ``` >You can check whether a control is legitimately disabled for a screen reader by visiting -[chrome://accessibility](chrome://accessibility). +[chrome://accessibility](chrome://accessibility) and inspecting [the accessibility tree](http://www.paciellogroup.com/blog/2015/01/the-browser-accessibility-tree/).

ngShow

@@ -210,16 +208,25 @@ 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.

ngClick and ngDblclick

-If `ng-click` or `ng-dblclick` is encountered, ngAria will add `tabindex="0"` if it isn't there -already. +If `ng-click` or `ng-dblclick` is encountered, ngAria will add `tabindex="0"` to any element not in +a node blacklist: -To fix widespread accessibility problems with `ng-click` on div elements, ngAria will dynamically -bind keypress by default as long as the element isn't an anchor, button, input or textarea. -You can turn this functionality on or off with the `bindKeypress` configuration option. ngAria -will also add the `button` role to communicate to users of assistive technologies. + * Button + * Anchor + * Input + * Textarea + * Select + * Details/Summary -For `ng-dblclick`, you must still manually add `ng-keypress` and role to non-interactive elements such -as `div` or `taco-button` to enable keyboard access. +To fix widespread accessibility problems with `ng-click` on `div` elements, ngAria will +dynamically bind a keypress event by default as long as the element isn't in the node blacklist. +You can turn this functionality on or off with the `bindKeypress` configuration option. + +ngAria will also add the `button` role to communicate to users of assistive technologies. This can +be disabled with the `bindRoleForClick` configuration option. + +For `ng-dblclick`, you must still manually add `ng-keypress` and a role to non-interactive elements +such as `div` or `taco-button` to enable keyboard access.

Example

```html @@ -260,62 +267,18 @@ The attribute magic of ngAria may not work for every scenario. To disable indivi you can use the {@link ngAria.$ariaProvider#config config} method. Just keep in mind this will tell ngAria to ignore the attribute globally. - + - -
-
- Div with ngModel and aria-invalid disabled -
-
- - Custom Checkbox for comparison +
+ <div> with ng-click and bindRoleForClick, tabindex set to false
-