fix(material/button): avoid setting a tabindex on all link buttons

Fixes that the button component was always setting a `tabindex="0"` when it's placed on a link element. This is unnecessary, because links are in the tab order by default.

We actually had a test that has an assertion against this, but it was passing by accident because we weren't running change detection before making the asserting.

Author: crisbeto

src/material-experimental/mdc-button/button-base.ts

@@ -141,7 +141,7 @@ export const MAT_ANCHOR_HOST = {
   // Note that we ignore the user-specified tabindex when it's disabled for
   // consistency with the `mat-button` applied on native buttons where even
   // though they have an index, they're not tabbable.
-  '[attr.tabindex]': 'disabled ? -1 : (tabIndex || 0)',
+  '[attr.tabindex]': 'disabled ? -1 : tabIndex',
   '[attr.aria-disabled]': 'disabled.toString()',
   // MDC automatically applies the primary theme color to the button, but we want to support
   // an unthemed version. If color is undefined, apply a CSS class that makes it easy to

src/material-experimental/mdc-button/button.spec.ts

@@ -169,11 +169,13 @@ describe('MDC-based MatButton', () => {
       let fixture = TestBed.createComponent(TestApp);
       let testComponent = fixture.debugElement.componentInstance;
       let buttonDebugElement = fixture.debugElement.query(By.css('a'))!;
-      expect(buttonDebugElement.nativeElement.getAttribute('tabIndex')).toBe(null);
+      fixture.detectChanges();
+
+      expect(buttonDebugElement.nativeElement.hasAttribute('tabindex')).toBe(false);
 
       testComponent.isDisabled = true;
       fixture.detectChanges();
-      expect(buttonDebugElement.nativeElement.getAttribute('tabIndex')).toBe('-1');
+      expect(buttonDebugElement.nativeElement.getAttribute('tabindex')).toBe('-1');
     });
 
     it('should add aria-disabled attribute if disabled', () => {
@@ -218,17 +220,25 @@ describe('MDC-based MatButton', () => {
       fixture.componentInstance.tabIndex = 3;
       fixture.detectChanges();
 
-      expect(buttonElement.getAttribute('tabIndex'))
+      expect(buttonElement.getAttribute('tabindex'))
         .withContext('Expected custom tabindex to be set')
         .toBe('3');
 
       testComponent.isDisabled = true;
       fixture.detectChanges();
 
-      expect(buttonElement.getAttribute('tabIndex'))
+      expect(buttonElement.getAttribute('tabindex'))
         .withContext('Expected custom tabindex to be overwritten when disabled.')
         .toBe('-1');
     });
+
+    it('should not set a default tabindex on enabled links', () => {
+      const fixture = TestBed.createComponent(TestApp);
+      const buttonElement = fixture.debugElement.query(By.css('a'))!.nativeElement;
+      fixture.detectChanges();
+
+      expect(buttonElement.hasAttribute('tabindex')).toBe(false);
+    });
   });
 
   // Ripple tests.

src/material/button/button.spec.ts

@@ -180,14 +180,14 @@ describe('MatButton', () => {
     });
 
     it('should remove tabindex if disabled', () => {
-      const fixture = TestBed.createComponent(TestApp);
-      const testComponent = fixture.debugElement.componentInstance;
-      const buttonDebugElement = fixture.debugElement.query(By.css('a'))!;
-      expect(buttonDebugElement.nativeElement.getAttribute('tabIndex')).toBe(null);
+      let fixture = TestBed.createComponent(TestApp);
+      let testComponent = fixture.debugElement.componentInstance;
+      let buttonDebugElement = fixture.debugElement.query(By.css('a'))!;
+      expect(buttonDebugElement.nativeElement.hasAttribute('tabindex')).toBe(false);
 
       testComponent.isDisabled = true;
       fixture.detectChanges();
-      expect(buttonDebugElement.nativeElement.getAttribute('tabIndex')).toBe('-1');
+      expect(buttonDebugElement.nativeElement.getAttribute('tabindex')).toBe('-1');
     });
 
     it('should add aria-disabled attribute if disabled', () => {
@@ -232,17 +232,25 @@ describe('MatButton', () => {
       fixture.componentInstance.tabIndex = 3;
       fixture.detectChanges();
 
-      expect(buttonElement.getAttribute('tabIndex'))
+      expect(buttonElement.getAttribute('tabindex'))
         .withContext('Expected custom tabindex to be set')
         .toBe('3');
 
       testComponent.isDisabled = true;
       fixture.detectChanges();
 
-      expect(buttonElement.getAttribute('tabIndex'))
+      expect(buttonElement.getAttribute('tabindex'))
         .withContext('Expected custom tabindex to be overwritten when disabled.')
         .toBe('-1');
     });
+
+    it('should not set a default tabindex on enabled links', () => {
+      const fixture = TestBed.createComponent(TestApp);
+      const buttonElement = fixture.debugElement.query(By.css('a'))!.nativeElement;
+      fixture.detectChanges();
+
+      expect(buttonElement.hasAttribute('tabindex')).toBe(false);
+    });
   });
 
   // Ripple tests.

src/material/button/button.ts

@@ -161,7 +161,7 @@ export class MatButton
     // Note that we ignore the user-specified tabindex when it's disabled for
     // consistency with the `mat-button` applied on native buttons where even
     // though they have an index, they're not tabbable.
-    '[attr.tabindex]': 'disabled ? -1 : (tabIndex || 0)',
+    '[attr.tabindex]': 'disabled ? -1 : tabIndex',
     '[attr.disabled]': 'disabled || null',
     '[attr.aria-disabled]': 'disabled.toString()',
     '(click)': '_haltDisabledEvents($event)',