From 65a1564ec9abdd8734268c8f870e218c38a597ad Mon Sep 17 00:00:00 2001
From: Kristiyan Kostadinov <crisbeto@abv.bg>
Date: Thu, 24 Feb 2022 14:19:18 +0100
Subject: [PATCH] fix(material/sort): remove role from header when disabled

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.
---
 src/material/sort/sort-header.html |  2 +-
 src/material/sort/sort-header.ts   |  2 +-
 src/material/sort/sort.spec.ts     | 10 +++++++---
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/material/sort/sort-header.html b/src/material/sort/sort-header.html
index 6d6067dfe085..77af0a462235 100644
--- a/src/material/sort/sort-header.html
+++ b/src/material/sort/sort-header.html
@@ -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
diff --git a/src/material/sort/sort-header.ts b/src/material/sort/sort-header.ts
index 72f1e125fe8b..c7594dba61fe 100644
--- a/src/material/sort/sort-header.ts
+++ b/src/material/sort/sort-header.ts
@@ -222,7 +222,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);
   }
 
diff --git a/src/material/sort/sort.spec.ts b/src/material/sort/sort.spec.ts
index f8ad12dec352..56d5cc2b873e 100644
--- a/src/material/sort/sort.spec.ts
+++ b/src/material/sort/sort.spec.ts
@@ -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', () => {
@@ -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();
 
@@ -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();