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

Commit

Permalink
fix(list): case where list items are read twice by screen readers
Browse files Browse the repository at this point in the history
- if we create an `aria-label` from the element's text content
  then mark the content as `aria-hidden="true"` so that screen
  readers don't announce/traverse the content twice
- improve Closure types

Fixes #11582
  • Loading branch information
Splaktar committed Sep 23, 2020
1 parent c644d6a commit 5c455d3
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 8 deletions.
19 changes: 12 additions & 7 deletions src/components/list/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,16 +343,21 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
);

// Button which shows ripple and executes primary action.
var buttonWrap = angular.element(
'<md-button class="md-no-style"></md-button>'
);
var buttonWrap = angular.element('<md-button class="md-no-style"></md-button>');

moveAttributes(tElement[0], buttonWrap[0]);

// If there is no aria-label set on the button (previously copied over if present)
// we determine the label from the content and copy it to the button.
if (!buttonWrap.attr('aria-label')) {
buttonWrap.attr('aria-label', $mdAria.getText(tElement));

// If we set the button's aria-label to the text content, then make the content hidden
// from screen readers so that it isn't read/traversed twice.
var listItemInner = itemContainer[0].querySelector('.md-list-item-inner');
if (listItemInner) {
listItemInner.setAttribute('aria-hidden', 'true');
}
}

// We allow developers to specify the `md-no-focus` class, to disable the focus style
Expand Down Expand Up @@ -386,7 +391,7 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {

/**
* @param {HTMLElement} secondaryItem
* @param container
* @param {HTMLDivElement} container
*/
function wrapSecondaryItem(secondaryItem, container) {
// If the current secondary item is not a button, but contains a ng-click attribute,
Expand Down Expand Up @@ -423,9 +428,9 @@ function mdListItemDirective($mdAria, $mdConstant, $mdUtil, $timeout) {
* Moves attributes from a source element to the destination element.
* By default, the function will copy the most necessary attributes, supported
* by the button executor for clickable list items.
* @param source Element with the specified attributes
* @param destination Element which will receive the attributes
* @param extraAttrs Additional attributes, which will be moved over
* @param {Element} source Element with the specified attributes
* @param {Element} destination Element which will receive the attributes
* @param {string|string[]} extraAttrs Additional attributes, which will be moved over
*/
function moveAttributes(source, destination, extraAttrs) {
var copiedAttrs = $mdUtil.prefixer([
Expand Down
4 changes: 4 additions & 0 deletions src/components/list/list.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -512,10 +512,12 @@ describe('mdListItem directive', function() {
it('should copy label to the button executor element', function() {
var listItem = setup('<md-list-item ng-click="null" aria-label="Test">');
var buttonEl = listItem.find('button');
var listItemInnerElement = listItem[0].querySelector('.md-list-item-inner');

// The aria-label attribute should be moved to the button element.
expect(buttonEl.attr('aria-label')).toBe('Test');
expect(listItem.attr('aria-label')).toBeFalsy();
expect(listItemInnerElement.getAttribute('aria-hidden')).toBeFalsy();
});

it('should determine the label from the content if not set', function() {
Expand All @@ -527,9 +529,11 @@ describe('mdListItem directive', function() {
);

var buttonEl = listItem.find('button');
var listItemInnerElement = listItem[0].querySelector('.md-list-item-inner');

// The aria-label attribute should be determined from the content.
expect(buttonEl.attr('aria-label')).toBe('Content');
expect(listItemInnerElement.getAttribute('aria-hidden')).toBeTruthy();
});

it('should determine the label from the bound content if aria-label is not set', function() {
Expand Down
10 changes: 9 additions & 1 deletion src/core/services/aria/aria.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,13 @@ function MdAriaService($$rAF, $log, $window, $interpolate) {
}
}

/**
* @param {Element|JQLite} element
* @returns {string}
*/
function getText(element) {
element = element[0] || element;
var walker = document.createTreeWalker(element, NodeFilter.SHOW_TEXT, null, false);
var walker = document.createTreeWalker(element, NodeFilter.SHOW_TEXT, null);
var text = '';

var node;
Expand All @@ -142,6 +146,10 @@ function MdAriaService($$rAF, $log, $window, $interpolate) {

return text.trim() || '';

/**
* @param {Node} node
* @returns {boolean}
*/
function isAriaHiddenNode(node) {
while (node.parentNode && (node = node.parentNode) !== element) {
if (node.getAttribute && node.getAttribute('aria-hidden') === 'true') {
Expand Down

0 comments on commit 5c455d3

Please sign in to comment.