This repository was archived by the owner on Apr 12, 2024. It is now read-only.
fix(ngAria): Apply ARIA attrs correctly #13483
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,19 +16,23 @@ | |
| * | ||
| * For ngAria to do its magic, simply include the module `ngAria` as a dependency. The following | ||
| * directives are supported: | ||
| * `ngModel`, `ngChecked`, `ngRequired`, `ngValue`, `ngDisabled`, `ngShow`, `ngHide`, `ngClick`, | ||
| * `ngDblClick`, and `ngMessages`. | ||
| * | ||
| * Below is a more detailed breakdown of the attributes handled by ngAria: | ||
| * | ||
| * | Directive | Supported Attributes | | ||
| * |---------------------------------------------|----------------------------------------------------------------------------------------| | ||
| * | {@link ng.directive:ngModel ngModel} | aria-checked, aria-valuemin, aria-valuemax, aria-valuenow, aria-invalid, aria-required, input roles | | ||
| * | {@link ng.directive:ngDisabled ngDisabled} | aria-disabled | | ||
| * | {@link ng.directive:ngRequired ngRequired} | aria-required | | ||
| * | {@link ng.directive:ngChecked ngChecked} | aria-checked | | ||
| * | {@link ng.directive:ngValue ngValue} | aria-checked | | ||
| * | {@link ng.directive:ngShow ngShow} | aria-hidden | | ||
| * | {@link ng.directive:ngHide ngHide} | aria-hidden | | ||
| * | {@link ng.directive:ngDblclick ngDblclick} | tabindex | | ||
| * | {@link module:ngMessages ngMessages} | aria-live | | ||
| * | {@link ng.directive:ngClick ngClick} | tabindex, keypress event, button role | | ||
| * | ||
| * Find out more information about each directive by reading the | ||
| * {@link guide/accessibility ngAria Developer Guide}. | ||
|
|
@@ -90,7 +94,6 @@ function $AriaProvider() { | |
| ariaDisabled: true, | ||
| ariaRequired: true, | ||
| ariaInvalid: true, | ||
| ariaValue: true, | ||
| tabindex: true, | ||
| bindKeypress: true, | ||
|
|
@@ -108,11 +111,10 @@ function $AriaProvider() { | |
| * - **ariaDisabled** – `{boolean}` – Enables/disables aria-disabled tags | ||
| * - **ariaRequired** – `{boolean}` – Enables/disables aria-required tags | ||
| * - **ariaInvalid** – `{boolean}` – Enables/disables aria-invalid tags | ||
| * - **ariaValue** – `{boolean}` – Enables/disables aria-valuemin, aria-valuemax and aria-valuenow tags | ||
| * - **tabindex** – `{boolean}` – Enables/disables tabindex tags | ||
| * - **bindKeypress** – `{boolean}` – Enables/disables keypress event binding on `div` and | ||
| * `li` elements with ng-click | ||
| * - **bindRoleForClick** – `{boolean}` – Adds role=button to non-interactive elements like `div` | ||
| * using ng-click, making them more accessible to users of assistive technologies | ||
| * | ||
|
|
@@ -151,28 +153,31 @@ function $AriaProvider() { | |
| * | ||
| *```js | ||
| * ngAriaModule.directive('ngDisabled', ['$aria', function($aria) { | ||
| * return $aria.$$watchExpr('ngDisabled', 'aria-disabled', nodeBlackList, false); | ||
| * }]) | ||
| *``` | ||
| * Shown above, the ngAria module creates a directive with the same signature as the | ||
| * traditional `ng-disabled` directive. But this ngAria version is dedicated to | ||
| * solely managing accessibility attributes on custom elements. The internal `$aria` service is | ||
| * used to watch the boolean attribute `ngDisabled`. If it has not been explicitly set by the | ||
| * developer, `aria-disabled` is injected as an attribute with its value synchronized to the | ||
| * value in `ngDisabled`. | ||
| * | ||
| * Because ngAria hooks into the `ng-disabled` directive, developers do not have to do | ||
| * anything to enable this feature. The `aria-disabled` attribute is automatically managed | ||
| * simply as a silent side-effect of using `ng-disabled` with the ngAria module. | ||
| * | ||
| * The full list of directives that interface with ngAria: | ||
| * * **ngModel** | ||
| * * **ngChecked** | ||
| * * **ngRequired** | ||
| * * **ngDisabled** | ||
| * * **ngValue** | ||
| * * **ngShow** | ||
| * * **ngHide** | ||
| * * **ngClick** | ||
| * * **ngDblclick** | ||
| * * **ngMessages** | ||
| * | ||
| * Read the {@link guide/accessibility ngAria Developer Guide} for a thorough explanation of each | ||
| * directive. | ||
|
|
@@ -198,13 +203,25 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { | |
| .directive('ngHide', ['$aria', function($aria) { | ||
| return $aria.$$watchExpr('ngHide', 'aria-hidden', [], false); | ||
| }]) | ||
| .directive('ngValue', ['$aria', function($aria) { | ||
| return $aria.$$watchExpr('ngValue', 'aria-checked', nodeBlackList, false); | ||
| }]) | ||
| .directive('ngChecked', ['$aria', function($aria) { | ||
| return $aria.$$watchExpr('ngChecked', 'aria-checked', nodeBlackList, false); | ||
| }]) | ||
| .directive('ngRequired', ['$aria', function($aria) { | ||
| return $aria.$$watchExpr('ngRequired', 'aria-required', nodeBlackList, false); | ||
| }]) | ||
| .directive('ngModel', ['$aria', '$parse', function($aria, $parse) { | ||
|
|
||
| function shouldAttachAttr(attr, normalizedAttr, elem, allowBlacklistEls) { | ||
| return $aria.config(normalizedAttr) && !elem.attr(attr) && (allowBlacklistEls || !isNodeOneOf(elem, nodeBlackList)); | ||
| } | ||
|
|
||
| function shouldAttachRole(role, elem) { | ||
| // if element does not have role attribute | ||
| // AND element type is equal to role (if custom element has a type equaling shape) <-- remove? | ||
| // AND element is not INPUT | ||
| return !elem.attr('role') && (elem.attr('type') === role) && (elem[0].nodeName !== 'INPUT'); | ||
| } | ||
|
|
||
|
|
@@ -214,8 +231,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { | |
|
|
||
| return ((type || role) === 'checkbox' || role === 'menuitemcheckbox') ? 'checkbox' : | ||
| ((type || role) === 'radio' || role === 'menuitemradio') ? 'radio' : | ||
| (type === 'range' || role === 'progressbar' || role === 'slider') ? 'range' : ''; | ||
| } | ||
|
|
||
| return { | ||
|
|
@@ -227,37 +243,26 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { | |
|
|
||
| return { | ||
| pre: function(scope, elem, attr, ngModel) { | ||
| if (shape === 'checkbox') { | ||
| //Use the input[checkbox] $isEmpty implementation for elements with checkbox roles | ||
| ngModel.$isEmpty = function(value) { | ||
| return value === false; | ||
| }; | ||
| } | ||
| }, | ||
| post: function(scope, elem, attr, ngModel) { | ||
| var needsTabIndex = shouldAttachAttr('tabindex', 'tabindex', elem, false); | ||
|
|
||
| function ngAriaWatchModelValue() { | ||
| return ngModel.$modelValue; | ||
| } | ||
|
|
||
| function getRadioReaction(newVal) { | ||
| var boolVal = (attr.value == ngModel.$viewValue); | ||
| elem.attr('aria-checked', boolVal); | ||
| } | ||
|
|
||
| function getCheckboxReaction() { | ||
|
There was a problem hiding this comment. This wrapper function doesn't seem to be necessary here. There was a problem hiding this comment. It was added to be consumed in a watch function on line 282. There was a problem hiding this comment. Why do you need to call a function that returns the actual function ? Just use |
||
| elem.attr('aria-checked', !ngModel.$isEmpty(ngModel.$viewValue)); | ||
| } | ||
|
|
||
|
|
@@ -267,9 +272,9 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { | |
| if (shouldAttachRole(shape, elem)) { | ||
| elem.attr('role', shape); | ||
| } | ||
| if (shouldAttachAttr('aria-checked', 'ariaChecked', elem, false)) { | ||
| scope.$watch(ngAriaWatchModelValue, shape === 'radio' ? | ||
| getRadioReaction : getCheckboxReaction); | ||
| } | ||
| if (needsTabIndex) { | ||
| elem.attr('tabindex', 0); | ||
|
|
@@ -306,22 +311,17 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { | |
| elem.attr('tabindex', 0); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| if (!attr.hasOwnProperty('ngRequired') && ngModel.$validators.required | ||
| && shouldAttachAttr('aria-required', 'ariaRequired', elem, false)) { | ||
| // ngModel.$error.required is undefined on custom controls | ||
| attr.$observe('required', function() { | ||
| elem.attr('aria-required', !!attr['required']); | ||
| }); | ||
| } | ||
|
|
||
| if (shouldAttachAttr('aria-invalid', 'ariaInvalid', elem, true)) { | ||
| scope.$watch(function ngAriaInvalidWatch() { | ||
| return ngModel.$invalid; | ||
| }, function ngAriaInvalidReaction(newVal) { | ||
|
|
@@ -334,7 +334,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { | |
| }; | ||
| }]) | ||
| .directive('ngDisabled', ['$aria', function($aria) { | ||
| return $aria.$$watchExpr('ngDisabled', 'aria-disabled', nodeBlackList, false); | ||
| }]) | ||
| .directive('ngMessages', function() { | ||
| return { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcysutton, why are we only checking against
input? Shouldn't all native controls (e.g.textarea,select,button) be excluded ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that line should be replaced with
isNodeOneOf(elem, nodeBlackList)). It has already gone through thegetShapefunction but that only checkstypeandrole.