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

fix(ngAria): clean up tabindex usage #12262

Closed
wants to merge 2 commits into from
Closed

fix(ngAria): clean up tabindex usage #12262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f129167
fix(ngAria): clean up tabindex usage
marcysutton Jul 2, 2015
fd15b73
doc(ngAria): Update accessibility guide
marcysutton Aug 4, 2015
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
97 changes: 30 additions & 67 deletions docs/content/guide/accessibility.ngdoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -41,8 +41,8 @@ Currently, ngAria interfaces with the following directives:

<h2 id="ngmodel">ngModel</h2>

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
Expand Down Expand Up @@ -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 `<div>` or `<md-checkbox>`: 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.

<h2 id="ngdisabled">ngDisabled</h2>

Expand All @@ -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/).

<h2 id="ngshow">ngShow</h2>

Expand Down Expand Up @@ -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.

<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="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.

<h3>Example</h3>
```html
Expand Down Expand Up @@ -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.

<example module="ngAria_ngDisabledExample" deps="angular-aria.js">
<example module="ngAria_ngClickExample" deps="angular-aria.js">
<file name="index.html">
<style>
[role=checkbox] {
cursor: pointer;
display: inline-block;
}
[role=checkbox] .icon:before {
content: '\2610';
display: inline-block;
font-size: 2em;
line-height: 1;
vertical-align: middle;
speak: none;
}
[role=checkbox].active .icon:before {
content: '\2611';
}
</style>
<form ng-controller="formsController">
<div ng-model="someModel" show-attrs>
Div with ngModel and aria-invalid disabled
</div>
<div role="checkbox" ng-model="checked" ng-class="{active: checked}"
aria-label="Custom Checkbox" ng-click="toggleCheckbox()" some-checkbox show-attrs>
<span class="icon" aria-hidden="true"></span>
Custom Checkbox for comparison
<div ng-click="someFunction" show-attrs>
&lt;div&gt; with ng-click and bindRoleForClick, tabindex set to false
</div>
</form>
<script>
angular.module('ngAria_ngDisabledExample', ['ngAria'], function config($ariaProvider) {
angular.module('ngAria_ngClickExample', ['ngAria'], function config($ariaProvider) {
$ariaProvider.config({
ariaInvalid: false,
tabindex: true
bindRoleForClick: false,
tabindex: false
});
})
.controller('formsController', function($scope){
$scope.checked = false;
$scope.toggleCheckbox = function(){
$scope.checked = !$scope.checked;
}
})
.directive('someCheckbox', function(){
return {
restrict: 'A',
link: function($scope, $el, $attrs) {
$el.on('keypress', function(event){
event.preventDefault();
if(event.keyCode === 32 || event.keyCode === 13){
$scope.toggleCheckbox();
$scope.$apply();
}
});
}
}
})
.directive('showAttrs', function() {
return function(scope, el, attrs) {
var pre = document.createElement('pre');
Expand Down
74 changes: 39 additions & 35 deletions src/ngAria/aria.js
View file
Open in desktop
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', []);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[] here is a placeholder for forthcoming ng-disabled improvements. This PR is purposefully limited.

}])
.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
Open in desktop
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');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided Angular and ngAria shouldn't be in the business of fixing custom radiogroups...it's too big of a can of worms when it comes to expected keyboard behavior.

});

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