From 7c4b1a3793251db3626d0cb80dbf9311422ca529 Mon Sep 17 00:00:00 2001
From: Miles Malerba <mmalerba@google.com>
Date: Mon, 15 May 2017 13:27:21 -0700
Subject: [PATCH 1/4] add cdk-focus-init

---
 src/demo-app/sidenav/sidenav-demo.html | 22 ++++++++++++
 src/lib/core/a11y/focus-trap.spec.ts   |  4 +--
 src/lib/core/a11y/focus-trap.ts        | 48 +++++++++++++++++++-------
 src/lib/dialog/dialog-container.ts     |  2 +-
 src/lib/sidenav/sidenav.ts             |  2 +-
 5 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/src/demo-app/sidenav/sidenav-demo.html b/src/demo-app/sidenav/sidenav-demo.html
index 3b362b3006d6..ba9d1593ba08 100644
--- a/src/demo-app/sidenav/sidenav-demo.html
+++ b/src/demo-app/sidenav/sidenav-demo.html
@@ -61,3 +61,25 @@ <h2>Dynamic Alignment Sidenav</h2>
     <button (click)="invert = !invert">Change sides</button>
   </div>
 </md-sidenav-container>
+
+<md-sidenav-container class="demo-sidenav-container">
+  <md-sidenav #focusSidenav>
+    <md-nav-list>
+      <a md-list-item routerLink>Link</a>
+      <a md-list-item routerLink cdk-focus-region-start>Focus region start</a>
+      <a md-list-item routerLink>Link</a>
+      <a md-list-item routerLink cdk-focus-init>Initially focused</a>
+      <a md-list-item routerLink cdk-focus-region-end>Focus region end</a>
+      <a md-list-item routerLink>Link</a>
+    </md-nav-list>
+  </md-sidenav>
+
+  <div class="demo-sidenav-content">
+    <h1>My Content</h1>
+
+    <div>
+      <header>Sidenav</header>
+      <button md-button (click)="focusSidenav.toggle()">Toggle Drawer</button>
+    </div>
+  </div>
+</md-sidenav-container>
diff --git a/src/lib/core/a11y/focus-trap.spec.ts b/src/lib/core/a11y/focus-trap.spec.ts
index 49900604f4ff..a05ab9391d18 100644
--- a/src/lib/core/a11y/focus-trap.spec.ts
+++ b/src/lib/core/a11y/focus-trap.spec.ts
@@ -167,8 +167,8 @@ class FocusTrapWithBindings {
   template: `
     <div cdkTrapFocus>
       <input>
-      <button id="last" cdk-focus-end></button>
-      <button id="first" cdk-focus-start>SAVE</button>
+      <button id="last" cdk-focus-region-end></button>
+      <button id="first" cdk-focus-region-start>SAVE</button>
       <input>
     </div>
     `
diff --git a/src/lib/core/a11y/focus-trap.ts b/src/lib/core/a11y/focus-trap.ts
index 8a993da72e6b..9793c4387f1b 100644
--- a/src/lib/core/a11y/focus-trap.ts
+++ b/src/lib/core/a11y/focus-trap.ts
@@ -81,6 +81,10 @@ export class FocusTrap {
     });
   }
 
+  focusInitialElementWhenReady() {
+    this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusInitialElement());
+  }
+
   /**
    * Waits for microtask queue to empty, then focuses
    * the first tabbable element within the focus trap region.
@@ -97,11 +101,39 @@ export class FocusTrap {
     this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusLastTabbableElement());
   }
 
+  private _getRegionMarker(marker: 'start' | 'end'): HTMLElement | null {
+    let markers = [
+      ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-region-${marker}]`)),
+      ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-${marker}]`)),
+    ];
+
+    markers.forEach((el: HTMLElement) => {
+      if (el.hasAttribute(`cdk-focus-${marker}`)) {
+        console.warn(`Found use of deprecated attribute 'cdk-focus-${marker}',` +
+                     ` use 'cdk-focus-region-${marker}' instead.`, el);
+      }
+    });
+
+    if (marker == 'start') {
+      return markers.length ? markers[0] : this._getFirstTabbableElement(this._element);
+    }
+    return markers.length ?
+        markers[markers.length - 1] : this._getLastTabbableElement(this._element);
+  }
+
+  /** Focuses the element that should be focused when the focus trap is initialized. */
+  focusInitialElement() {
+    let redirectToElement = this._element.querySelector('[cdk-focus-init]') as HTMLElement;
+    if (redirectToElement) {
+      redirectToElement.focus();
+    } else {
+      this.focusFirstTabbableElement();
+    }
+  }
+
   /** Focuses the first tabbable element within the focus trap region. */
   focusFirstTabbableElement() {
-    let redirectToElement = this._element.querySelector('[cdk-focus-start]') as HTMLElement ||
-                            this._getFirstTabbableElement(this._element);
-
+    let redirectToElement = this._getRegionMarker('start');
     if (redirectToElement) {
       redirectToElement.focus();
     }
@@ -109,15 +141,7 @@ export class FocusTrap {
 
   /** Focuses the last tabbable element within the focus trap region. */
   focusLastTabbableElement() {
-    let focusTargets = this._element.querySelectorAll('[cdk-focus-end]');
-    let redirectToElement: HTMLElement = null;
-
-    if (focusTargets.length) {
-      redirectToElement = focusTargets[focusTargets.length - 1] as HTMLElement;
-    } else {
-      redirectToElement = this._getLastTabbableElement(this._element);
-    }
-
+    let redirectToElement = this._getRegionMarker('end');
     if (redirectToElement) {
       redirectToElement.focus();
     }
diff --git a/src/lib/dialog/dialog-container.ts b/src/lib/dialog/dialog-container.ts
index acb5c1459b99..9dcc9d7686c7 100644
--- a/src/lib/dialog/dialog-container.ts
+++ b/src/lib/dialog/dialog-container.ts
@@ -118,7 +118,7 @@ export class MdDialogContainer extends BasePortalHost {
     // If were to attempt to focus immediately, then the content of the dialog would not yet be
     // ready in instances where change detection has to run first. To deal with this, we simply
     // wait for the microtask queue to be empty.
-    this._focusTrap.focusFirstTabbableElementWhenReady();
+    this._focusTrap.focusInitialElementWhenReady();
   }
 
   /** Restores focus to the element that was focused before the dialog opened. */
diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts
index fa1acb75f05d..bb86c2311307 100644
--- a/src/lib/sidenav/sidenav.ts
+++ b/src/lib/sidenav/sidenav.ts
@@ -133,7 +133,7 @@ export class MdSidenav implements AfterContentInit, OnDestroy {
       this._elementFocusedBeforeSidenavWasOpened = document.activeElement as HTMLElement;
 
       if (this.isFocusTrapEnabled && this._focusTrap) {
-        this._focusTrap.focusFirstTabbableElementWhenReady();
+        this._focusTrap.focusInitialElementWhenReady();
       }
     });
 

From 93480946c36206b82b31f9b44c9a9ea5dbab0c65 Mon Sep 17 00:00:00 2001
From: Miles Malerba <mmalerba@google.com>
Date: Mon, 15 May 2017 15:37:05 -0700
Subject: [PATCH 2/4] fix nav list styling

---
 src/demo-app/sidenav/sidenav-demo.html | 2 ++
 src/lib/list/_list-theme.scss          | 4 +++-
 src/lib/list/list-item.html            | 2 +-
 src/lib/list/list.ts                   | 9 ++++-----
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/demo-app/sidenav/sidenav-demo.html b/src/demo-app/sidenav/sidenav-demo.html
index ba9d1593ba08..f8197912feca 100644
--- a/src/demo-app/sidenav/sidenav-demo.html
+++ b/src/demo-app/sidenav/sidenav-demo.html
@@ -62,6 +62,8 @@ <h2>Dynamic Alignment Sidenav</h2>
   </div>
 </md-sidenav-container>
 
+<h2>Sidenav with focus attributes</h2>
+
 <md-sidenav-container class="demo-sidenav-container">
   <md-sidenav #focusSidenav>
     <md-nav-list>
diff --git a/src/lib/list/_list-theme.scss b/src/lib/list/_list-theme.scss
index 1b630dcbc523..d839c1912f72 100644
--- a/src/lib/list/_list-theme.scss
+++ b/src/lib/list/_list-theme.scss
@@ -20,7 +20,9 @@
     border-top-color: mat-color($foreground, divider);
   }
 
-  .mat-nav-list .mat-list-item-content {
+  .mat-nav-list .mat-list-item {
+    outline: none;
+
     &:hover, &.mat-list-item-focus {
       background: mat-color($background, 'hover');
     }
diff --git a/src/lib/list/list-item.html b/src/lib/list/list-item.html
index 16c46abd0a10..9eb276ea7304 100644
--- a/src/lib/list/list-item.html
+++ b/src/lib/list/list-item.html
@@ -1,4 +1,4 @@
-<div class="mat-list-item-content" [class.mat-list-item-focus]="_hasFocus">
+<div class="mat-list-item-content">
   <div class="mat-list-item-ripple" md-ripple
        [mdRippleTrigger]="_getHostElement()"
        [mdRippleDisabled]="!isRippleEnabled()">
diff --git a/src/lib/list/list.ts b/src/lib/list/list.ts
index 11fb7c4c288b..73513412d3f3 100644
--- a/src/lib/list/list.ts
+++ b/src/lib/list/list.ts
@@ -9,7 +9,7 @@ import {
   Input,
   Optional,
   Renderer2,
-  AfterContentInit,
+  AfterContentInit, ChangeDetectorRef,
 } from '@angular/core';
 import {MdLine, MdLineSetter, coerceBooleanProperty} from '../core';
 
@@ -128,8 +128,6 @@ export class MdListItem implements AfterContentInit {
   private _disableRipple: boolean = false;
   private _isNavList: boolean = false;
 
-  _hasFocus: boolean = false;
-
   /**
    * Whether the ripple effect on click should be disabled. This applies only to list items that are
    * part of a nav list. The value of `disableRipple` on the `md-nav-list` overrides this flag.
@@ -151,6 +149,7 @@ export class MdListItem implements AfterContentInit {
 
   constructor(private _renderer: Renderer2,
               private _element: ElementRef,
+              private _cdr: ChangeDetectorRef,
               @Optional() private _list: MdList,
               @Optional() navList: MdNavListCssMatStyler) {
     this._isNavList = !!navList;
@@ -166,11 +165,11 @@ export class MdListItem implements AfterContentInit {
   }
 
   _handleFocus() {
-    this._hasFocus = true;
+    this._renderer.addClass(this._element.nativeElement, 'mat-list-item-focus');
   }
 
   _handleBlur() {
-    this._hasFocus = false;
+    this._renderer.removeClass(this._element.nativeElement, 'mat-list-item-focus');
   }
 
   /** Retrieves the DOM element of the component host. */

From 11ac2e2379ca1840c41303195ed68d2d5fa3acc9 Mon Sep 17 00:00:00 2001
From: Miles Malerba <mmalerba@google.com>
Date: Mon, 15 May 2017 15:49:52 -0700
Subject: [PATCH 3/4] add test

---
 src/lib/core/a11y/focus-trap.spec.ts | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/lib/core/a11y/focus-trap.spec.ts b/src/lib/core/a11y/focus-trap.spec.ts
index a05ab9391d18..f2a066f7e54c 100644
--- a/src/lib/core/a11y/focus-trap.spec.ts
+++ b/src/lib/core/a11y/focus-trap.spec.ts
@@ -14,7 +14,7 @@ describe('FocusTrap', () => {
         FocusTrapWithBindings,
         SimpleFocusTrap,
         FocusTrapTargets,
-        FocusTrapWithSvg
+        FocusTrapWithSvg,
       ],
       providers: [InteractivityChecker, Platform, FocusTrapFactory]
     });
@@ -104,6 +104,13 @@ describe('FocusTrap', () => {
       focusTrapInstance = fixture.componentInstance.focusTrapDirective.focusTrap;
     });
 
+    it('should be able to set initial focus target', () => {
+      // Because we can't mimic a real tab press focus change in a unit test, just call the
+      // focus event handler directly.
+      focusTrapInstance.focusInitialElement();
+      expect(document.activeElement.id).toBe('middle');
+    });
+
     it('should be able to prioritize the first focus target', () => {
       // Because we can't mimic a real tab press focus change in a unit test, just call the
       // focus event handler directly.
@@ -131,7 +138,6 @@ describe('FocusTrap', () => {
       expect(() => focusTrapInstance.focusLastTabbableElement()).not.toThrow();
     });
   });
-
 });
 
 
@@ -168,6 +174,7 @@ class FocusTrapWithBindings {
     <div cdkTrapFocus>
       <input>
       <button id="last" cdk-focus-region-end></button>
+      <button id="middle" cdk-focus-init></button>
       <button id="first" cdk-focus-region-start>SAVE</button>
       <input>
     </div>

From 41ce7ecf1a125de06adb24a44184a581590c5112 Mon Sep 17 00:00:00 2001
From: Miles Malerba <mmalerba@google.com>
Date: Mon, 15 May 2017 17:01:05 -0700
Subject: [PATCH 4/4] address comments

---
 src/demo-app/sidenav/sidenav-demo.html |  2 +-
 src/lib/core/a11y/focus-trap.spec.ts   |  6 ++++--
 src/lib/core/a11y/focus-trap.ts        | 26 ++++++++++++++++----------
 src/lib/list/list.spec.ts              | 11 ++++++-----
 src/lib/list/list.ts                   | 11 +++++------
 5 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/src/demo-app/sidenav/sidenav-demo.html b/src/demo-app/sidenav/sidenav-demo.html
index f8197912feca..10da18a0fa31 100644
--- a/src/demo-app/sidenav/sidenav-demo.html
+++ b/src/demo-app/sidenav/sidenav-demo.html
@@ -70,7 +70,7 @@ <h2>Sidenav with focus attributes</h2>
       <a md-list-item routerLink>Link</a>
       <a md-list-item routerLink cdk-focus-region-start>Focus region start</a>
       <a md-list-item routerLink>Link</a>
-      <a md-list-item routerLink cdk-focus-init>Initially focused</a>
+      <a md-list-item routerLink cdk-focus-initial>Initially focused</a>
       <a md-list-item routerLink cdk-focus-region-end>Focus region end</a>
       <a md-list-item routerLink>Link</a>
     </md-nav-list>
diff --git a/src/lib/core/a11y/focus-trap.spec.ts b/src/lib/core/a11y/focus-trap.spec.ts
index f2a066f7e54c..66e42601212b 100644
--- a/src/lib/core/a11y/focus-trap.spec.ts
+++ b/src/lib/core/a11y/focus-trap.spec.ts
@@ -173,9 +173,11 @@ class FocusTrapWithBindings {
   template: `
     <div cdkTrapFocus>
       <input>
+      <button>before</button>
+      <button id="first" cdk-focus-region-start></button>
+      <button id="middle" cdk-focus-initial></button>
       <button id="last" cdk-focus-region-end></button>
-      <button id="middle" cdk-focus-init></button>
-      <button id="first" cdk-focus-region-start>SAVE</button>
+      <button>after</button>
       <input>
     </div>
     `
diff --git a/src/lib/core/a11y/focus-trap.ts b/src/lib/core/a11y/focus-trap.ts
index 9793c4387f1b..1d853ae21ed1 100644
--- a/src/lib/core/a11y/focus-trap.ts
+++ b/src/lib/core/a11y/focus-trap.ts
@@ -101,20 +101,26 @@ export class FocusTrap {
     this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusLastTabbableElement());
   }
 
-  private _getRegionMarker(marker: 'start' | 'end'): HTMLElement | null {
+  /**
+   * Get the specified boundary element of the trapped region.
+   * @param bound The boundary to get (start or end of trapped region).
+   * @returns The boundary element.
+   */
+  private _getRegionBoundary(bound: 'start' | 'end'): HTMLElement | null {
     let markers = [
-      ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-region-${marker}]`)),
-      ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-${marker}]`)),
+      ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-region-${bound}]`)),
+      // Deprecated version of selector, for temporary backwards comparability:
+      ...Array.prototype.slice.call(this._element.querySelectorAll(`[cdk-focus-${bound}]`)),
     ];
 
     markers.forEach((el: HTMLElement) => {
-      if (el.hasAttribute(`cdk-focus-${marker}`)) {
-        console.warn(`Found use of deprecated attribute 'cdk-focus-${marker}',` +
-                     ` use 'cdk-focus-region-${marker}' instead.`, el);
+      if (el.hasAttribute(`cdk-focus-${bound}`)) {
+        console.warn(`Found use of deprecated attribute 'cdk-focus-${bound}',` +
+                     ` use 'cdk-focus-region-${bound}' instead.`, el);
       }
     });
 
-    if (marker == 'start') {
+    if (bound == 'start') {
       return markers.length ? markers[0] : this._getFirstTabbableElement(this._element);
     }
     return markers.length ?
@@ -123,7 +129,7 @@ export class FocusTrap {
 
   /** Focuses the element that should be focused when the focus trap is initialized. */
   focusInitialElement() {
-    let redirectToElement = this._element.querySelector('[cdk-focus-init]') as HTMLElement;
+    let redirectToElement = this._element.querySelector('[cdk-focus-initial]') as HTMLElement;
     if (redirectToElement) {
       redirectToElement.focus();
     } else {
@@ -133,7 +139,7 @@ export class FocusTrap {
 
   /** Focuses the first tabbable element within the focus trap region. */
   focusFirstTabbableElement() {
-    let redirectToElement = this._getRegionMarker('start');
+    let redirectToElement = this._getRegionBoundary('start');
     if (redirectToElement) {
       redirectToElement.focus();
     }
@@ -141,7 +147,7 @@ export class FocusTrap {
 
   /** Focuses the last tabbable element within the focus trap region. */
   focusLastTabbableElement() {
-    let redirectToElement = this._getRegionMarker('end');
+    let redirectToElement = this._getRegionBoundary('end');
     if (redirectToElement) {
       redirectToElement.focus();
     }
diff --git a/src/lib/list/list.spec.ts b/src/lib/list/list.spec.ts
index b266b38517b1..5bc594c0b38d 100644
--- a/src/lib/list/list.spec.ts
+++ b/src/lib/list/list.spec.ts
@@ -28,18 +28,19 @@ describe('MdList', () => {
 
   it('should add and remove focus class on focus/blur', () => {
     let fixture = TestBed.createComponent(ListWithOneAnchorItem);
-    let listItem = fixture.debugElement.query(By.directive(MdListItem));
-    let listItemDiv = fixture.debugElement.query(By.css('.mat-list-item-content'));
     fixture.detectChanges();
-    expect(listItemDiv.nativeElement.classList).not.toContain('mat-list-item-focus');
+    let listItem = fixture.debugElement.query(By.directive(MdListItem));
+    let listItemEl = fixture.debugElement.query(By.css('.mat-list-item'));
+
+    expect(listItemEl.nativeElement.classList).not.toContain('mat-list-item-focus');
 
     listItem.componentInstance._handleFocus();
     fixture.detectChanges();
-    expect(listItemDiv.nativeElement.classList).toContain('mat-list-item-focus');
+    expect(listItemEl.nativeElement.classList).toContain('mat-list-item-focus');
 
     listItem.componentInstance._handleBlur();
     fixture.detectChanges();
-    expect(listItemDiv.nativeElement.classList).not.toContain('mat-list-item-focus');
+    expect(listItemEl.nativeElement.classList).not.toContain('mat-list-item-focus');
   });
 
   it('should not apply any additional class to a list without lines', () => {
diff --git a/src/lib/list/list.ts b/src/lib/list/list.ts
index 73513412d3f3..a8e7501be1d5 100644
--- a/src/lib/list/list.ts
+++ b/src/lib/list/list.ts
@@ -1,17 +1,17 @@
 import {
+  AfterContentInit,
   Component,
-  ViewEncapsulation,
-  ContentChildren,
   ContentChild,
-  QueryList,
+  ContentChildren,
   Directive,
   ElementRef,
   Input,
   Optional,
+  QueryList,
   Renderer2,
-  AfterContentInit, ChangeDetectorRef,
+  ViewEncapsulation
 } from '@angular/core';
-import {MdLine, MdLineSetter, coerceBooleanProperty} from '../core';
+import {coerceBooleanProperty, MdLine, MdLineSetter} from '../core';
 
 @Directive({
   selector: 'md-divider, mat-divider'
@@ -149,7 +149,6 @@ export class MdListItem implements AfterContentInit {
 
   constructor(private _renderer: Renderer2,
               private _element: ElementRef,
-              private _cdr: ChangeDetectorRef,
               @Optional() private _list: MdList,
               @Optional() navList: MdNavListCssMatStyler) {
     this._isNavList = !!navList;