Skip to content

Commit

Permalink
fix(material/sort): remove role from header when disabled (#24477)
Browse files Browse the repository at this point in the history
When the sort header is disabled, we make it non-interactive by clearing its `tabindex`, however it still has the `role="button"` which can throw off screen readers if it doesn't have text.

These changes also clear the `role` from the header if it's disabled.

(cherry picked from commit 229dd6e)
  • Loading branch information
crisbeto committed Feb 24, 2022
1 parent 6c9221c commit 1a498a6
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/material/sort/sort-header.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
[class.mat-sort-header-sorted]="_isSorted()"
[class.mat-sort-header-position-before]="arrowPosition == 'before'"
[attr.tabindex]="_isDisabled() ? null : 0"
role="button">
[attr.role]="_isDisabled() ? null : 'button'">

<!--
TODO(crisbeto): this div isn't strictly necessary, but we have to keep it due to a large
Expand Down
2 changes: 1 addition & 1 deletion src/material/sort/sort-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export class MatSortHeader

this._sort.register(this);

this._sortButton = this._elementRef.nativeElement.querySelector('[role="button"]')!;
this._sortButton = this._elementRef.nativeElement.querySelector('.mat-sort-header-container')!;
this._updateSortActionDescription(this._sortActionDescription);
}

Expand Down
10 changes: 7 additions & 3 deletions src/material/sort/sort.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,13 +237,15 @@ describe('MatSort', () => {
component.sort('defaultA');
expect(component.matSort.direction).toBe('asc');
expect(container.getAttribute('tabindex')).toBe('0');
expect(container.getAttribute('role')).toBe('button');

component.disabledColumnSort = true;
fixture.detectChanges();

component.sort('defaultA');
expect(component.matSort.direction).toBe('asc');
expect(container.getAttribute('tabindex')).toBeFalsy();
expect(container.hasAttribute('tabindex')).toBe(false);
expect(container.hasAttribute('role')).toBe(false);
});

it('should allow for the cycling the sort direction to be disabled for all columns', () => {
Expand Down Expand Up @@ -417,7 +419,7 @@ describe('MatSort', () => {
});

it('should add a default aria description to sort buttons', () => {
const sortButton = fixture.nativeElement.querySelector('[role="button"]');
const sortButton = fixture.nativeElement.querySelector('.mat-sort-header-container');
const descriptionId = sortButton.getAttribute('aria-describedby');
expect(descriptionId).toBeDefined();

Expand All @@ -426,7 +428,9 @@ describe('MatSort', () => {
});

it('should add a custom aria description to sort buttons', () => {
const sortButton = fixture.nativeElement.querySelector('#defaultB [role="button"]');
const sortButton = fixture.nativeElement.querySelector(
'#defaultB .mat-sort-header-container',
);
let descriptionId = sortButton.getAttribute('aria-describedby');
expect(descriptionId).toBeDefined();

Expand Down

0 comments on commit 1a498a6

Please sign in to comment.