Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

Commit 90c8b8d

Browse files
committed
fix(list): secondary actions give aria warning if text is in a span
- an `md-list-item`'s text should be in a `<span>` or `<p>` but currently the `setupToggleAria()` function only handles `<p>` this adds support for `<span>` - add JSDoc - fix typos in comments - add docs for localizing aria labels on secondary actions Fixes #6152
1 parent 0ab73bd commit 90c8b8d

File tree

2 files changed

+65
-28
lines changed

2 files changed

+65
-28
lines changed

src/components/list/list.js

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,13 @@ function mdListDirective($mdTheming) {
136136
* </md-list-item>
137137
* </hljs>
138138
*
139-
* The `md-checkbox` element will be automatically detected as a proxy element and will toggle on click.
139+
* The `md-checkbox` element will be automatically detected as a proxy element and will toggle on
140+
* click.
141+
*
142+
* If not provided, an `aria-label` will be applied using the text of the list item.
143+
* In this case, the following will be applied to the `md-checkbox`:
144+
* `aria-label="Toggle First Line"`.
145+
* When localizing your application, you should supply a localized `aria-label`.
140146
*
141147
* <hljs lang="html">
142148
* <md-list-item>
@@ -220,12 +226,12 @@ function mdListDirective($mdTheming) {
220226
* because otherwise some elements may not be clickable inside of the button.
221227
*
222228
* ---
223-
* When using a secondary item inside of your list item, the `md-list-item` component will automatically create
224-
* a secondary container at the end of the `md-list-item`, which contains all secondary items.
225-
*
226-
* The secondary item container is not static, because otherwise the overflow will not work properly on the
227-
* list item.
229+
* When using a secondary item inside of your list item, the `md-list-item` component will
230+
* automatically create a secondary container at the end of the `md-list-item`, which contains all
231+
* secondary items.
228232
*
233+
* The secondary item container is not static, because that would cause issues with the overflow
234+
* of the list item.
229235
*/
230236
function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
231237
var proxiedTypes = ['md-checkbox', 'md-switch', 'md-menu'];
@@ -260,7 +266,6 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
260266
} else {
261267
tElement.addClass('md-no-proxy');
262268
}
263-
264269
}
265270

266271
wrapSecondaryItems();
@@ -278,9 +283,12 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
278283
toggle = tElement.find(toggleTypes[i])[0];
279284
if (toggle) {
280285
if (!toggle.hasAttribute('aria-label')) {
281-
var p = tElement.find('p')[0];
282-
if (!p) return;
283-
toggle.setAttribute('aria-label', 'Toggle ' + p.textContent);
286+
var labelElement = tElement.find('p')[0];
287+
if (!labelElement) {
288+
labelElement = tElement.find('span')[0];
289+
}
290+
if (!labelElement) return;
291+
toggle.setAttribute('aria-label', 'Toggle ' + labelElement.textContent);
284292
}
285293
}
286294
}
@@ -326,8 +334,8 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
326334
} else {
327335
// Element which holds the default list-item content.
328336
itemContainer = angular.element(
329-
'<div class="md-button md-no-style">'+
330-
' <div class="md-list-item-inner"></div>'+
337+
'<div class="md-button md-no-style">' +
338+
' <div class="md-list-item-inner"></div>' +
331339
'</div>'
332340
);
333341

@@ -372,6 +380,10 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
372380
itemContainer.append(secondaryItemsWrapper);
373381
}
374382

383+
/**
384+
* @param {HTMLElement} secondaryItem
385+
* @param container
386+
*/
375387
function wrapSecondaryItem(secondaryItem, container) {
376388
// If the current secondary item is not a button, but contains a ng-click attribute,
377389
// the secondary item will be automatically wrapped inside of a button.
@@ -391,9 +403,11 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
391403
secondaryItem = buttonWrapper[0];
392404
}
393405

394-
if (secondaryItem && (!hasClickEvent(secondaryItem) || (!tAttrs.ngClick && isProxiedElement(secondaryItem)))) {
395-
// In this case we remove the secondary class, so we can identify it later, when we searching for the
396-
// proxy items.
406+
if (secondaryItem &&
407+
(!hasClickEvent(secondaryItem) ||
408+
(!tAttrs.ngClick && isProxiedElement(secondaryItem)))) {
409+
// In this case we remove the secondary class, so we can identify it later, when searching
410+
// for the proxy items.
397411
angular.element(secondaryItem).removeClass('md-secondary');
398412
}
399413

@@ -427,17 +441,29 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
427441
});
428442
}
429443

430-
function isProxiedElement(el) {
431-
return proxiedTypes.indexOf(el.nodeName.toLowerCase()) !== -1;
444+
/**
445+
* @param {HTMLElement} element
446+
* @return {boolean} true if the element has one of the proxied tags, false otherwise
447+
*/
448+
function isProxiedElement(element) {
449+
return proxiedTypes.indexOf(element.nodeName.toLowerCase()) !== -1;
432450
}
433451

434-
function isButton(el) {
435-
var nodeName = el.nodeName.toUpperCase();
452+
/**
453+
* @param {HTMLElement} element
454+
* @return {boolean} true if the element is a button or md-button, false otherwise
455+
*/
456+
function isButton(element) {
457+
var nodeName = element.nodeName.toUpperCase();
436458

437459
return nodeName === "MD-BUTTON" || nodeName === "BUTTON";
438460
}
439461

440-
function hasClickEvent (element) {
462+
/**
463+
* @param {Element} element
464+
* @return {boolean} true if the element has an ng-click attribute, false otherwise
465+
*/
466+
function hasClickEvent(element) {
441467
var attr = element.attributes;
442468
for (var i = 0; i < attr.length; i++) {
443469
if (tAttrs.$normalize(attr[i].name) === 'ngClick') return true;
@@ -467,7 +493,7 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
467493
$scope.mouseActive = false;
468494
proxy.on('mousedown', function() {
469495
$scope.mouseActive = true;
470-
$timeout(function(){
496+
$timeout(function() {
471497
$scope.mouseActive = false;
472498
}, 100);
473499
})
@@ -481,20 +507,16 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
481507
});
482508
}
483509

484-
485510
function computeProxies() {
486-
487511
if (firstElement && firstElement.children && !hasClick && !noProxies) {
488512

489513
angular.forEach(proxiedTypes, function(type) {
490-
491-
// All elements which are not capable for being used a proxy have the .md-secondary class
492-
// applied. These items had been sorted out in the secondary wrap function.
514+
// All elements which are not capable of being used as a proxy have the .md-secondary
515+
// class applied. These items were identified in the secondary wrap function.
493516
angular.forEach(firstElement.querySelectorAll(type + ':not(.md-secondary)'), function(child) {
494517
proxies.push(child);
495518
});
496519
});
497-
498520
}
499521
}
500522

@@ -586,7 +608,6 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
586608
* @ngdoc controller
587609
* @name MdListController
588610
* @module material.components.list
589-
*
590611
*/
591612
function MdListController($scope, $element, $mdListInkRipple) {
592613
var ctrl = this;

src/components/list/list.spec.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,22 @@ describe('mdListItem directive', function() {
548548
expect(buttonEl.attr('aria-label')).toBe('Content');
549549
});
550550

551+
it('should determine the toggle label from the content if not set', function() {
552+
var listItem = setup(
553+
'<md-list-item ng-click="null">' +
554+
' <span>Content</span>' +
555+
' <md-switch class="md-secondary" ng-model="isContentOn"></md-switch>' +
556+
'</md-list-item>'
557+
);
558+
559+
var buttonElement = listItem.find('button');
560+
var switchElement = listItem.find('md-switch');
561+
562+
// The aria-label attribute should be determined from the content.
563+
expect(buttonElement.attr('aria-label')).toBe('Content');
564+
expect(switchElement.attr('aria-label')).toBe('Toggle Content');
565+
});
566+
551567
it('should warn when label is missing and content is empty', inject(function($log) {
552568
// Clear the log stack to assert that a new warning has been added.
553569
$log.reset();

0 commit comments

Comments
 (0)