From c3d96724bd446daa76638a98a6adf70f4d9ebba8 Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Fri, 29 Sep 2017 15:28:40 +0200
Subject: [PATCH] fix(selection-list): tabIndex should respect disabled state

* Currently if the selection-list is disabled, the tabIndex may be still set to a valid value that allows tabbing to the element. The `mixinTabIndex` respects the disabled state of the selection list.
---
 src/lib/list/selection-list.spec.ts | 56 +++++++++++++++++++++++++++++
 src/lib/list/selection-list.ts      | 25 +++++++------
 2 files changed, 68 insertions(+), 13 deletions(-)

diff --git a/src/lib/list/selection-list.spec.ts b/src/lib/list/selection-list.spec.ts
index 457d0b1532ea..fedb39f3af1b 100644
--- a/src/lib/list/selection-list.spec.ts
+++ b/src/lib/list/selection-list.spec.ts
@@ -219,6 +219,49 @@ describe('MatSelectionList', () => {
     });
   });
 
+  describe('with tabindex', () => {
+
+    beforeEach(async(() => {
+      TestBed.configureTestingModule({
+        imports: [MatListModule],
+        declarations: [
+          SelectionListWithTabindexAttr,
+          SelectionListWithTabindexBinding,
+        ]
+      });
+
+      TestBed.compileComponents();
+    }));
+
+    it('should properly handle native tabindex attribute', () => {
+      const fixture = TestBed.createComponent(SelectionListWithTabindexAttr);
+      const selectionList = fixture.debugElement.query(By.directive(MatSelectionList));
+
+      expect(selectionList.componentInstance.tabIndex)
+        .toBe(5, 'Expected the selection-list tabindex to be set to the attribute value.');
+    });
+
+    it('should support changing the tabIndex through binding', () => {
+      const fixture = TestBed.createComponent(SelectionListWithTabindexBinding);
+      const selectionList = fixture.debugElement.query(By.directive(MatSelectionList));
+
+      expect(selectionList.componentInstance.tabIndex)
+        .toBe(0, 'Expected the tabIndex to be set to "0" by default.');
+
+      fixture.componentInstance.tabIndex = 3;
+      fixture.detectChanges();
+
+      expect(selectionList.componentInstance.tabIndex)
+        .toBe(3, 'Expected the tabIndex to updated through binding.');
+
+      fixture.componentInstance.disabled = true;
+      fixture.detectChanges();
+
+      expect(selectionList.componentInstance.tabIndex)
+        .toBe(-1, 'Expected the tabIndex to be set to "-1" if selection list is disabled.');
+    });
+  });
+
   describe('with single option', () => {
     let fixture: ComponentFixture<SelectionListWithOnlyOneOption>;
     let listOption: DebugElement;
@@ -464,3 +507,16 @@ class SelectionListWithDisabledOption {
   </mat-selection-list>`})
 class SelectionListWithOnlyOneOption {
 }
+
+@Component({
+  template: `<mat-selection-list tabindex="5"></mat-selection-list>`
+})
+class SelectionListWithTabindexAttr {}
+
+@Component({
+  template: `<mat-selection-list [tabIndex]="tabIndex" [disabled]="disabled"></mat-selection-list>`
+})
+class SelectionListWithTabindexBinding {
+  tabIndex: number;
+  disabled: boolean;
+}
diff --git a/src/lib/list/selection-list.ts b/src/lib/list/selection-list.ts
index 3f939fa600c7..9e81920ab07c 100644
--- a/src/lib/list/selection-list.ts
+++ b/src/lib/list/selection-list.ts
@@ -28,14 +28,17 @@ import {
   QueryList,
   Renderer2,
   ViewEncapsulation,
+  Attribute,
 } from '@angular/core';
 import {
   CanDisable,
   CanDisableRipple,
   MatLine,
   MatLineSetter,
+  HasTabIndex,
   mixinDisabled,
   mixinDisableRipple,
+  mixinTabIndex,
 } from '@angular/material/core';
 import {merge} from 'rxjs/observable/merge';
 import {Subscription} from 'rxjs/Subscription';
@@ -43,7 +46,8 @@ import {Subscription} from 'rxjs/Subscription';
 
 /** @docs-private */
 export class MatSelectionListBase {}
-export const _MatSelectionListMixinBase = mixinDisableRipple(mixinDisabled(MatSelectionListBase));
+export const _MatSelectionListMixinBase =
+  mixinTabIndex(mixinDisableRipple(mixinDisabled(MatSelectionListBase)));
 
 /** @docs-private */
 export class MatListOptionBase {}
@@ -187,10 +191,10 @@ export class MatListOption extends _MatListOptionMixinBase
 @Component({
   moduleId: module.id,
   selector: 'mat-selection-list',
-  inputs: ['disabled', 'disableRipple'],
+  inputs: ['disabled', 'disableRipple', 'tabIndex'],
   host: {
     'role': 'listbox',
-    '[attr.tabindex]': '_tabIndex',
+    '[tabIndex]': 'tabIndex',
     'class': 'mat-selection-list',
     '(focus)': 'focus()',
     '(keydown)': '_keydown($event)',
@@ -201,11 +205,8 @@ export class MatListOption extends _MatListOptionMixinBase
   preserveWhitespaces: false,
   changeDetection: ChangeDetectionStrategy.OnPush
 })
-export class MatSelectionList extends _MatSelectionListMixinBase
-    implements FocusableOption, CanDisable, CanDisableRipple, AfterContentInit, OnDestroy {
-
-  /** Tab index for the selection-list. */
-  _tabIndex = 0;
+export class MatSelectionList extends _MatSelectionListMixinBase implements FocusableOption,
+    CanDisable, CanDisableRipple, HasTabIndex, AfterContentInit, OnDestroy {
 
   /** Subscription to all list options' onFocus events */
   private _optionFocusSubscription = Subscription.EMPTY;
@@ -222,17 +223,15 @@ export class MatSelectionList extends _MatSelectionListMixinBase
   /** The currently selected options. */
   selectedOptions: SelectionModel<MatListOption> = new SelectionModel<MatListOption>(true);
 
-  constructor(private _element: ElementRef) {
+  constructor(private _element: ElementRef, @Attribute('tabindex') tabIndex: string) {
     super();
+
+    this.tabIndex = parseInt(tabIndex) || 0;
   }
 
   ngAfterContentInit(): void {
     this._keyManager = new FocusKeyManager<MatListOption>(this.options).withWrap();
 
-    if (this.disabled) {
-      this._tabIndex = -1;
-    }
-
     this._optionFocusSubscription = this._onFocusSubscription();
     this._optionDestroyStream = this._onDestroySubscription();
   }