From dc901760353aeb6d4480af1c3a8d31d55bdbdbf7 Mon Sep 17 00:00:00 2001 From: Michael Prentice Date: Wed, 29 Jul 2020 21:59:58 -0400 Subject: [PATCH] fix(radio-button): when no value selected, first button not announced on focus - when no value selected, set `aria-activedescendant` to first button's `id` - properly define labels and `aria-labelledby` in demos - improve some JSDoc and types - rename some variables to make them more readable - remove unused parameters - improvements to multi-column demo - remove unneeded or unused styles and elements - use `aria-describedby` to make first column available to screen reader users Fixes #11973. Fixes #11923. Fixes #11974. --- .../dialog/demoBasicUsage/script.js | 2 +- .../radioButton/demoBasicUsage/index.html | 30 ++++---- .../radioButton/demoBasicUsage/script.js | 2 - .../radioButton/demoBasicUsage/style.css | 17 +---- .../radioButton/demoMultiColumn/index.html | 49 ++++--------- .../radioButton/demoMultiColumn/script.js | 2 +- .../radioButton/demoMultiColumn/style.css | 56 ++++---------- src/components/radioButton/radio-button.js | 73 +++++++++++++------ src/core/util/iterator.js | 9 ++- 9 files changed, 103 insertions(+), 137 deletions(-) diff --git a/src/components/dialog/demoBasicUsage/script.js b/src/components/dialog/demoBasicUsage/script.js index 7d3e8289e9..0c6ec05099 100644 --- a/src/components/dialog/demoBasicUsage/script.js +++ b/src/components/dialog/demoBasicUsage/script.js @@ -79,7 +79,7 @@ angular.module('dialogDemo1', ['ngMaterial']) templateUrl: 'tabDialog.tmpl.html', parent: angular.element(document.body), targetEvent: ev, - clickOutsideToClose:true + clickOutsideToClose: true }) .then(function(answer) { $scope.status = 'You said the information was "' + answer + '".'; diff --git a/src/components/radioButton/demoBasicUsage/index.html b/src/components/radioButton/demoBasicUsage/index.html index c3a063175c..556735f8dc 100644 --- a/src/components/radioButton/demoBasicUsage/index.html +++ b/src/components/radioButton/demoBasicUsage/index.html @@ -1,21 +1,19 @@
-

Selected Value: {{ data.group1 }}

- - - +

+ {{ data.group1 }} +

+ Apple Banana Mango - -
-

Selected Value: {{ data.group2 }}

- - - +

+ {{ data.group2 }} +

+ - - -

Add Remove

-
-

Graphic radio buttons need to be labeled with the aria-label attribute.

-

Selected Avatar: {{ data.group3 }}

+

Graphic radio buttons need to be labeled with the aria-label attribute.

+

+ {{ data.group3 }} +

- + diff --git a/src/components/radioButton/demoBasicUsage/script.js b/src/components/radioButton/demoBasicUsage/script.js index c57d08235e..6a5ce413d7 100644 --- a/src/components/radioButton/demoBasicUsage/script.js +++ b/src/components/radioButton/demoBasicUsage/script.js @@ -1,4 +1,3 @@ - angular .module('radioDemo1', ['ngMaterial']) .controller('AppCtrl', function($scope) { @@ -30,7 +29,6 @@ angular { label: '4', value: '4' } ]; - $scope.submit = function() { alert('submit'); }; diff --git a/src/components/radioButton/demoBasicUsage/style.css b/src/components/radioButton/demoBasicUsage/style.css index 42abf7e981..4d093399ff 100644 --- a/src/components/radioButton/demoBasicUsage/style.css +++ b/src/components/radioButton/demoBasicUsage/style.css @@ -1,42 +1,33 @@ - body { padding: 20px; } - hr { margin-left: -20px; opacity: 0.3; } - md-radio-group { width: 150px; } - p:last-child { padding-bottom: 50px; } - - -[ng-controller] { +form { padding: 0 20px; } - .radioValue { margin-left: 5px; color: #0f9d58; font-weight: bold; - padding:5px; - } - md-icon { margin: 0 20px 20px; width: 128px; height: 128px; } - - .ipsum { color: saddlebrown; font-size: 0.9em; } +.demo-description { + margin-bottom: 0; +} diff --git a/src/components/radioButton/demoMultiColumn/index.html b/src/components/radioButton/demoMultiColumn/index.html index f3a7624752..df5e6f8613 100644 --- a/src/components/radioButton/demoMultiColumn/index.html +++ b/src/components/radioButton/demoMultiColumn/index.html @@ -1,40 +1,23 @@ -
+

Contact List

- + - -
-
- -
- {{person.title}} -
- - - {{person.fullName}} - - + +
+
+
+ {{person.title}}
-
- - - -
-
- Selected User: {{cc.selectedUser()}} + + {{person.fullName}} +
-
- - + + +

+ {{ctrl.selectedUser()}} +

+
diff --git a/src/components/radioButton/demoMultiColumn/script.js b/src/components/radioButton/demoMultiColumn/script.js index d82cdc3d7e..986dcf799b 100644 --- a/src/components/radioButton/demoMultiColumn/script.js +++ b/src/components/radioButton/demoMultiColumn/script.js @@ -1,5 +1,5 @@ angular - .module('radioDemo2', ['ngMaterial']) + .module('radioMultiColumnDemo', ['ngMaterial']) .controller('ContactController', function($scope, $filter) { var self = this; diff --git a/src/components/radioButton/demoMultiColumn/style.css b/src/components/radioButton/demoMultiColumn/style.css index 6952c615f1..74f068f2b1 100644 --- a/src/components/radioButton/demoMultiColumn/style.css +++ b/src/components/radioButton/demoMultiColumn/style.css @@ -1,53 +1,23 @@ -div.radioDemo2 { - margin-bottom: 20px; -} - - h2 { - margin-left : 15px; -} -p { - width:400px; - margin-top:10px; - margin-left : 10px; - padding-top:10px; - border-top: 2px solid #ddd; + margin: 16px; } - -.md-checked { - background-color : #ECFAFB; +.demo-checked { + background-color: #ECFAFB; border-radius: 2px; } - -md-button.md-raised, button.md-raised { - width: 200px; -} - -.row { - border-bottom: 1px dashed #ddd; +.demo-row { + border-bottom: 1px dashed #ddd; } - -div.row:last-child { +.demo-row:last-child { border-bottom: 0px dashed #ddd; } - - - -.summary { - width: 100%; - padding-top:10px; - margin-left: 25px; - margin-top: 20px; - margin-bottom:-5px -} - - -.title { +label { font-weight: bolder; } - -.selectedUser .md-checked { - padding : 8px; - width: 100px; - +.demo-title { + flex: 0 1 200px; +} +html[dir="rtl"] .demo-bidi { + padding-right: 20px; + padding-left: 0; } diff --git a/src/components/radioButton/radio-button.js b/src/components/radioButton/radio-button.js index 5a3edb87be..4e2abcacd9 100644 --- a/src/components/radioButton/radio-button.js +++ b/src/components/radioButton/radio-button.js @@ -54,15 +54,15 @@ function mdRadioGroupDirective($mdUtil, $mdConstant, $mdTheming, $timeout) { link: { pre: linkRadioGroup } }; - function linkRadioGroup(scope, element, attr, ctrls) { + function linkRadioGroup(scope, element, attr, controllers) { // private md component indicator for styling element.addClass('_md'); $mdTheming(element); - var rgCtrl = ctrls[0]; - var ngModelCtrl = ctrls[1] || $mdUtil.fakeNgModel(); + var radioGroupController = controllers[0]; + var ngModelCtrl = controllers[1] || $mdUtil.fakeNgModel(); - rgCtrl.init(ngModelCtrl); + radioGroupController.init(ngModelCtrl); scope.mouseActive = false; @@ -72,7 +72,7 @@ function mdRadioGroupDirective($mdUtil, $mdConstant, $mdTheming, $timeout) { 'tabIndex': element.attr('tabindex') || '0' }) .on('keydown', keydownListener) - .on('mousedown', function(event) { + .on('mousedown', function() { scope.mouseActive = true; $timeout(function() { scope.mouseActive = false; @@ -80,13 +80,24 @@ function mdRadioGroupDirective($mdUtil, $mdConstant, $mdTheming, $timeout) { }) .on('focus', function() { if (scope.mouseActive === false) { - rgCtrl.$element.addClass('md-focused'); + radioGroupController.$element.addClass('md-focused'); } }) .on('blur', function() { - rgCtrl.$element.removeClass('md-focused'); + radioGroupController.$element.removeClass('md-focused'); }); + // Initially set the first radio button as the aria-activedescendant. This will be overridden + // if a 'checked' radio button gets rendered. We need to wait for the nextTick here so that the + // radio buttons have their id values assigned. + $mdUtil.nextTick(function () { + var radioButtons = getRadioButtons(radioGroupController.$element); + if (radioButtons.count() && + !radioGroupController.$element[0].hasAttribute('aria-activedescendant')) { + radioGroupController.setActiveDescendant(radioButtons.first().id); + } + }); + /** * Apply the md-focused class if it isn't already applied. */ @@ -111,14 +122,14 @@ function mdRadioGroupDirective($mdUtil, $mdConstant, $mdTheming, $timeout) { case $mdConstant.KEY_CODE.LEFT_ARROW: case $mdConstant.KEY_CODE.UP_ARROW: ev.preventDefault(); - rgCtrl.selectPrevious(); + radioGroupController.selectPrevious(); setFocus(); break; case $mdConstant.KEY_CODE.RIGHT_ARROW: case $mdConstant.KEY_CODE.DOWN_ARROW: ev.preventDefault(); - rgCtrl.selectNext(); + radioGroupController.selectNext(); setFocus(); break; @@ -132,6 +143,10 @@ function mdRadioGroupDirective($mdUtil, $mdConstant, $mdTheming, $timeout) { } } + /** + * @param {JQLite} $element + * @constructor + */ function RadioGroupController($element) { this._radioButtonRenderFns = []; this.$element = $element; @@ -179,13 +194,25 @@ function mdRadioGroupDirective($mdUtil, $mdConstant, $mdTheming, $timeout) { } }; } + + /** + * Coerce all child radio buttons into an array, then wrap then in an iterator + * @param parent {!JQLite} + * @return {{add: function(*=, *=): *, next: Function, last: function(): *, previous: Function, count: function(): (Array.length|*|number), hasNext: function(*=): (Array.length|*|number|boolean), inRange: function(*): (Array.length|*|number|boolean), remove: function(*=): void, contains: function(*=): boolean, itemAt: function(*=): *, findBy: function(*, *): Array, hasPrevious: function(*=): (Array.length|*|number|boolean), items: function(): (Array|*), indexOf: function(*=): *, first: function(): *}|Object|*|AsyncIterableIterator>>} + */ + function getRadioButtons(parent) { + return $mdUtil.iterator(parent[0].querySelectorAll('md-radio-button'), true); + } + /** * Change the radio group's selected button by a given increment. * If no button is selected, select the first button. + * @param {JQLite} parent + * @param {-1|1} increment select previous button if the value is negative; the next button + * otherwise. */ function changeSelectedButton(parent, increment) { - // Coerce all child radio buttons into an array, then wrap then in an iterator - var buttons = $mdUtil.iterator(parent[0].querySelectorAll('md-radio-button'), true); + var buttons = getRadioButtons(parent); if (buttons.count()) { var validate = function (button) { @@ -194,13 +221,13 @@ function mdRadioGroupDirective($mdUtil, $mdConstant, $mdTheming, $timeout) { }; var selected = parent[0].querySelector('md-radio-button.md-checked'); - var target = buttons[increment < 0 ? 'previous' : 'next'](selected, validate) || buttons.first(); + var target = buttons[increment < 0 ? 'previous' : 'next'](selected, validate) || + buttons.first(); // Activate radioButton's click listener (triggerHandler won't create a real click event) angular.element(target).triggerHandler('click'); } } - } /** @@ -257,11 +284,11 @@ function mdRadioButtonDirective($mdAria, $mdUtil, $mdTheming) { link: link }; - function link(scope, element, attr, rgCtrl) { + function link(scope, element, attr, radioGroupController) { var lastChecked; $mdTheming(element); - configureAria(element, scope); + configureAria(element); element.addClass('md-auto-horizontal-margin'); // ngAria overwrites the aria-checked inside a $watch for ngValue. @@ -278,17 +305,17 @@ function mdRadioButtonDirective($mdAria, $mdUtil, $mdTheming) { * Initializes the component. */ function initialize() { - if (!rgCtrl) { + if (!radioGroupController) { throw 'RadioButton: No RadioGroupController could be found.'; } - rgCtrl.add(render); + radioGroupController.add(render); attr.$observe('value', render); element .on('click', listener) .on('$destroy', function() { - rgCtrl.remove(render); + radioGroupController.remove(render); }); } @@ -296,10 +323,10 @@ function mdRadioButtonDirective($mdAria, $mdUtil, $mdTheming) { * On click functionality. */ function listener(ev) { - if (element[0].hasAttribute('disabled') || rgCtrl.isDisabled()) return; + if (element[0].hasAttribute('disabled') || radioGroupController.isDisabled()) return; scope.$apply(function() { - rgCtrl.setViewValue(attr.value, ev && ev.type); + radioGroupController.setViewValue(attr.value, ev && ev.type); }); } @@ -308,7 +335,7 @@ function mdRadioButtonDirective($mdAria, $mdUtil, $mdTheming) { * Update the `aria-activedescendant` attribute. */ function render() { - var checked = rgCtrl.getViewValue() == attr.value; + var checked = radioGroupController.getViewValue() == attr.value; if (checked === lastChecked) return; @@ -318,7 +345,7 @@ function mdRadioButtonDirective($mdAria, $mdUtil, $mdTheming) { } if (checked) { - rgCtrl.setActiveDescendant(element.attr('id')); + radioGroupController.setActiveDescendant(element.attr('id')); } lastChecked = checked; @@ -331,7 +358,7 @@ function mdRadioButtonDirective($mdAria, $mdUtil, $mdTheming) { /** * Inject ARIA-specific attributes appropriate for each radio button */ - function configureAria(element, scope){ + function configureAria(element) { element.attr({ id: attr.id || 'radio_' + $mdUtil.nextUid(), role: 'radio', diff --git a/src/core/util/iterator.js b/src/core/util/iterator.js index 4c5bb6ab53..abde6551c2 100644 --- a/src/core/util/iterator.js +++ b/src/core/util/iterator.js @@ -14,10 +14,12 @@ }); /** - * iterator is a list facade to easily support iteration and accessors + * iterator is a list facade to easily support iteration and accessors/ * - * @param items Array list which this iterator will enumerate - * @param reloop Boolean enables iterator to consider the list as an endless reloop + * @param {any[]} items Array list which this iterator will enumerate + * @param {boolean=} reloop enables iterator to consider the list as an endless loop + * @return {{add: add, next: (function()), last: (function(): any|null), previous: (function()), count: (function(): number), hasNext: (function(*=): Array.length|*|number|boolean), inRange: (function(*): boolean), remove: remove, contains: (function(*=): *|boolean), itemAt: (function(*=): any|null), findBy: (function(*, *): *[]), hasPrevious: (function(*=): Array.length|*|number|boolean), items: (function(): *[]), indexOf: (function(*=): number), first: (function(): any|null)}} + * @constructor */ function MdIterator(items, reloop) { var trueFn = function() { return true; }; @@ -51,7 +53,6 @@ hasPrevious: hasPrevious, hasNext: hasNext - }; /**