From 2106509739f6910347558d17867fe7773647b700 Mon Sep 17 00:00:00 2001
From: Kara Erickson <karakara@google.com>
Date: Wed, 31 Aug 2016 18:45:05 -0700
Subject: [PATCH 1/3] feat(menu): add keyboard events and accessibility

---
 e2e/components/menu/menu-page.ts  | 13 +++++
 e2e/components/menu/menu.e2e.ts   | 84 +++++++++++++++++++++++++++++--
 src/demo-app/menu/menu-demo.ts    |  4 +-
 src/e2e-app/menu/menu-e2e.html    |  2 +
 src/lib/core/keyboard/keycodes.ts | 13 +++++
 src/lib/menu/README.md            |  9 ++--
 src/lib/menu/menu-directive.ts    | 81 +++++++++++++++++++++++++++--
 src/lib/menu/menu-item.ts         | 37 ++++++--------
 src/lib/menu/menu-trigger.ts      | 56 +++++++++++++++++++--
 src/lib/menu/menu.html            |  2 +-
 src/lib/menu/menu.scss            |  2 +-
 src/lib/menu/menu.ts              |  8 +--
 src/lib/tabs/tabs.ts              |  9 +---
 13 files changed, 267 insertions(+), 53 deletions(-)
 create mode 100644 src/lib/core/keyboard/keycodes.ts

diff --git a/e2e/components/menu/menu-page.ts b/e2e/components/menu/menu-page.ts
index 25c4bf01d4e3..7e25d774004c 100644
--- a/e2e/components/menu/menu-page.ts
+++ b/e2e/components/menu/menu-page.ts
@@ -8,6 +8,8 @@ export class MenuPage {
 
   menu() { return element(by.css('.md-menu')); }
 
+  start() { return element(by.id('start')); }
+
   trigger() { return element(by.id('trigger')); }
 
   triggerTwo() { return element(by.id('trigger-two')); }
@@ -32,6 +34,17 @@ export class MenuPage {
 
   combinedMenu() { return element(by.css('.md-menu.combined')); }
 
+  // TODO(kara): move to common testing utility
+  pressKey(key: any): void {
+    browser.actions().sendKeys(key).perform();
+  }
+
+  // TODO(kara): move to common testing utility
+  expectFocusOn(el: ElementFinder): void {
+    expect(browser.driver.switchTo().activeElement().getInnerHtml())
+        .toBe(el.getInnerHtml());
+  }
+
   expectMenuPresent(expected: boolean) {
     return browser.isElementPresent(by.css('.md-menu')).then((isPresent) => {
       expect(isPresent).toBe(expected);
diff --git a/e2e/components/menu/menu.e2e.ts b/e2e/components/menu/menu.e2e.ts
index fda43f6f8349..b247bc113ef8 100644
--- a/e2e/components/menu/menu.e2e.ts
+++ b/e2e/components/menu/menu.e2e.ts
@@ -12,7 +12,7 @@ describe('menu', () => {
     page.trigger().click();
 
     page.expectMenuPresent(true);
-    expect(page.menu().getText()).toEqual("One\nTwo\nThree");
+    expect(page.menu().getText()).toEqual("One\nTwo\nThree\nFour");
   });
 
   it('should close menu when area outside menu is clicked', () => {
@@ -45,14 +45,14 @@ describe('menu', () => {
 
   it('should support multiple triggers opening the same menu', () => {
     page.triggerTwo().click();
-    expect(page.menu().getText()).toEqual("One\nTwo\nThree");
+    expect(page.menu().getText()).toEqual("One\nTwo\nThree\nFour");
     page.expectMenuAlignedWith(page.menu(), 'trigger-two');
 
     page.body().click();
     page.expectMenuPresent(false);
 
     page.trigger().click();
-    expect(page.menu().getText()).toEqual("One\nTwo\nThree");
+    expect(page.menu().getText()).toEqual("One\nTwo\nThree\nFour");
     page.expectMenuAlignedWith(page.menu(), 'trigger');
 
     page.body().click();
@@ -66,6 +66,84 @@ describe('menu', () => {
     });
   });
 
+  describe('keyboard events', () => {
+    beforeEach(() => {
+      // click start button to avoid tabbing past navigation
+      page.start().click();
+      page.pressKey(protractor.Key.TAB);
+    });
+
+    it('should auto-focus the first item when opened with keyboard', () => {
+      page.pressKey(protractor.Key.ENTER);
+      page.expectFocusOn(page.items(0));
+    });
+
+    it('should not focus the first item when opened with mouse', () => {
+      page.trigger().click();
+      page.expectFocusOn(page.trigger());
+    });
+
+    it('should focus subsequent items when down arrow is pressed', () => {
+      page.pressKey(protractor.Key.ENTER);
+      page.pressKey(protractor.Key.DOWN);
+      page.expectFocusOn(page.items(1));
+    });
+
+    it('should focus previous items when up arrow is pressed', () => {
+      page.pressKey(protractor.Key.ENTER);
+      page.pressKey(protractor.Key.DOWN);
+      page.pressKey(protractor.Key.UP);
+      page.expectFocusOn(page.items(0));
+    });
+
+    it('should skip disabled items using arrow keys', () => {
+      page.pressKey(protractor.Key.ENTER);
+      page.pressKey(protractor.Key.DOWN);
+      page.pressKey(protractor.Key.DOWN);
+      page.expectFocusOn(page.items(3));
+
+      page.pressKey(protractor.Key.UP);
+      page.expectFocusOn(page.items(1));
+    });
+
+    it('should close the menu when tabbing past items', () => {
+      page.pressKey(protractor.Key.ENTER);
+      page.pressKey(protractor.Key.TAB);
+      page.expectMenuPresent(false);
+
+      page.start().click();
+      page.pressKey(protractor.Key.TAB);
+      page.pressKey(protractor.Key.ENTER);
+      page.pressKey(protractor.Key.chord(protractor.Key.SHIFT, protractor.Key.TAB));
+      page.expectMenuPresent(false);
+    });
+
+    it('should wrap back to menu when arrow keying past items', () => {
+      page.pressKey(protractor.Key.ENTER);
+      page.pressKey(protractor.Key.DOWN);
+      page.pressKey(protractor.Key.DOWN);
+      page.pressKey(protractor.Key.DOWN);
+      page.expectFocusOn(page.items(0));
+
+      page.pressKey(protractor.Key.UP);
+      page.expectFocusOn(page.items(3));
+    });
+
+    it('should focus before and after trigger when tabbing past items', () => {
+      page.pressKey(protractor.Key.ENTER);
+      page.pressKey(protractor.Key.TAB);
+      page.expectFocusOn(page.triggerTwo());
+
+      // navigate back to trigger
+      page.pressKey(protractor.Key.chord(protractor.Key.SHIFT, protractor.Key.TAB));
+      page.pressKey(protractor.Key.ENTER);
+
+      page.pressKey(protractor.Key.chord(protractor.Key.SHIFT, protractor.Key.TAB));
+      page.expectFocusOn(page.start());
+    });
+
+  });
+
   describe('position - ', () => {
 
     it('should default menu alignment to "after below" when not set', () => {
diff --git a/src/demo-app/menu/menu-demo.ts b/src/demo-app/menu/menu-demo.ts
index 1d0d04bba99e..aafd42606ce5 100644
--- a/src/demo-app/menu/menu-demo.ts
+++ b/src/demo-app/menu/menu-demo.ts
@@ -12,8 +12,8 @@ export class MenuDemo {
   items = [
     {text: 'Refresh'},
     {text: 'Settings'},
-    {text: 'Help'},
-    {text: 'Sign Out', disabled: true}
+    {text: 'Help', disabled: true},
+    {text: 'Sign Out'}
   ];
 
   select(text: string) { this.selected = text; }
diff --git a/src/e2e-app/menu/menu-e2e.html b/src/e2e-app/menu/menu-e2e.html
index b77eee83e80a..057517b9c8fe 100644
--- a/src/e2e-app/menu/menu-e2e.html
+++ b/src/e2e-app/menu/menu-e2e.html
@@ -1,6 +1,7 @@
 <div>
   <div style="float:left">
     <div id="text">{{ selected }}</div>
+    <button id="start">START</button>
     <button [md-menu-trigger-for]="menu" id="trigger">TRIGGER</button>
     <button [md-menu-trigger-for]="menu" id="trigger-two">TRIGGER 2</button>
 
@@ -8,6 +9,7 @@
       <button md-menu-item (click)="selected='one'">One</button>
       <button md-menu-item (click)="selected='two'">Two</button>
       <button md-menu-item (click)="selected='three'" disabled>Three</button>
+      <button md-menu-item>Four</button>
     </md-menu>
 
     <button [md-menu-trigger-for]="beforeMenu" id="before-t">
diff --git a/src/lib/core/keyboard/keycodes.ts b/src/lib/core/keyboard/keycodes.ts
new file mode 100644
index 000000000000..df5a0aae4c12
--- /dev/null
+++ b/src/lib/core/keyboard/keycodes.ts
@@ -0,0 +1,13 @@
+
+// Due to a bug in the ChromeDriver, Angular 2 keyboard events are not triggered by `sendKeys`
+// during E2E tests when using dot notation such as `(keydown.rightArrow)`. To get around this,
+// we are temporarily using a single (keydown) handler.
+// See: https://github.com/angular/angular/issues/9419
+
+export const UP_ARROW = 38;
+export const DOWN_ARROW = 40;
+export const RIGHT_ARROW = 39;
+export const LEFT_ARROW = 37;
+
+export const ENTER = 13;
+export const TAB = 9;
diff --git a/src/lib/menu/README.md b/src/lib/menu/README.md
index 3ff0b69e4109..9ccd1c988cab 100644
--- a/src/lib/menu/README.md
+++ b/src/lib/menu/README.md
@@ -5,7 +5,6 @@
 
 ### Not yet implemented
 
-- Keyboard events: up arrow, down arrow, enter
 - `prevent-close` option, to turn off automatic menu close when clicking outside the menu
 - Custom offset support
 - Menu groupings (which menus are allowed to open together)
@@ -129,7 +128,12 @@ Output:
 ### Accessibility
 
 The menu adds `role="menu"` to the main menu element and `role="menuitem"` to each menu item. It 
-also adds `aria-hasPopup="true"` to the trigger element. 
+also adds `aria-hasPopup="true"` to the trigger element.
+
+#### Keyboard events:
+- <kbd>DOWN_ARROW</kbd> or <kbd>TAB</kbd>: Focus next menu item
+- <kbd>UP_ARROW</kbd> or <kbd>SHIFT_TAB</kbd>: Focus previous menu item
+- <kbd>ENTER</kbd>: Select focused item
 
 ### Menu attributes
 
@@ -158,4 +162,3 @@ also adds `aria-hasPopup="true"` to the trigger element.
 | `destroyMenu()` | `Promise<void>` | Destroys the menu overlay completely. 
   
 
-
diff --git a/src/lib/menu/menu-directive.ts b/src/lib/menu/menu-directive.ts
index 19f4ac721892..b96dba0c2f6d 100644
--- a/src/lib/menu/menu-directive.ts
+++ b/src/lib/menu/menu-directive.ts
@@ -1,18 +1,21 @@
-// TODO(kara): keyboard events for menu navigation
 // TODO(kara): prevent-close functionality
 
 import {
   Attribute,
   Component,
+  ContentChildren,
   EventEmitter,
   Input,
   Output,
+  QueryList,
   TemplateRef,
   ViewChild,
   ViewEncapsulation
 } from '@angular/core';
 import {MenuPositionX, MenuPositionY} from './menu-positions';
 import {MdMenuInvalidPositionX, MdMenuInvalidPositionY} from './menu-errors';
+import {MdMenuItem} from './menu-item';
+import {UP_ARROW, DOWN_ARROW, TAB} from '@angular2-material/core/keyboard/keycodes';
 
 @Component({
   moduleId: module.id,
@@ -25,6 +28,7 @@ import {MdMenuInvalidPositionX, MdMenuInvalidPositionY} from './menu-errors';
 })
 export class MdMenu {
   _showClickCatcher: boolean = false;
+  private _focusedItemIndex: number = 0;
 
   // config object to be passed into the menu's ngClass
   _classList: Object;
@@ -33,6 +37,7 @@ export class MdMenu {
   positionY: MenuPositionY = 'below';
 
   @ViewChild(TemplateRef) templateRef: TemplateRef<any>;
+  @ContentChildren(MdMenuItem) items: QueryList<MdMenuItem>;
 
   constructor(@Attribute('x-position') posX: MenuPositionX,
               @Attribute('y-position') posY: MenuPositionY) {
@@ -65,6 +70,76 @@ export class MdMenu {
     this._showClickCatcher = bool;
   }
 
+  /**
+   * Focus the first item in the menu. This method is used by the menu trigger
+   * to focus the first item when the menu is opened by the ENTER key.
+   * TODO: internal
+   */
+  _focusFirstItem() {
+    this.items.first.focus();
+  }
+
+  // TODO(kara): update this when (keydown.downArrow) testability is fixed
+  // TODO: internal
+  _handleKeydown(event: KeyboardEvent): void {
+    if (event.keyCode === DOWN_ARROW) {
+      this._focusNextItem();
+    } else if (event.keyCode === UP_ARROW) {
+      this._focusPreviousItem();
+    } else if (event.keyCode === TAB) {
+      this._emitCloseEvent();
+    }
+  }
+
+  /**
+   * This emits a close event to which the trigger is subscribed. When emitted, the
+   * trigger will close the menu.
+   */
+  private _emitCloseEvent(): void {
+    this._focusedItemIndex = 0;
+    this.close.emit(null);
+  }
+
+  // When focus would shift past the start or end of the menu, wrap back to the beginning or end
+  private _wrapIfFocusLeavesMenu(): void {
+    if (this._focusedItemIndex >= this.items.length) {
+      this._focusedItemIndex = 0;
+    } else if (this._focusedItemIndex < 0) {
+      this._focusedItemIndex = this.items.length - 1;
+    }
+  }
+
+  private _focusNextItem(): void {
+    this._updateFocusedItemIndex(1);
+    this.items.toArray()[this._focusedItemIndex].focus();
+  }
+
+  private _focusPreviousItem(): void {
+    this._updateFocusedItemIndex(-1);
+    this.items.toArray()[this._focusedItemIndex].focus();
+  }
+
+  /**
+   * This method sets focus to the correct menu item, given a list of menu items and the delta
+   * between the currently focused menu item and the new menu item to be focused. It will
+   * continue to move down the list until it finds an item that is not disabled, and it will wrap
+   * if it encounters either end of the menu.
+   *
+   * @param delta the desired change in focus index
+   * @param menuItems the menu items that should be focused
+   * @private
+     */
+  private _updateFocusedItemIndex(delta: number, menuItems: MdMenuItem[] = this.items.toArray()) {
+    this._focusedItemIndex += delta;
+    this._wrapIfFocusLeavesMenu();
+
+    // skip all disabled menu items recursively until an active one
+    // is reached or the menu closes for overreaching bounds
+    while (menuItems[this._focusedItemIndex].disabled) {
+      this._updateFocusedItemIndex(delta, menuItems);
+    }
+  }
+
   private _setPositionX(pos: MenuPositionX): void {
     if ( pos !== 'before' && pos !== 'after') {
       throw new MdMenuInvalidPositionX();
@@ -78,8 +153,4 @@ export class MdMenu {
     }
     this.positionY = pos;
   }
-
-  private _emitCloseEvent(): void {
-    this.close.emit(null);
-  }
 }
diff --git a/src/lib/menu/menu-item.ts b/src/lib/menu/menu-item.ts
index 28b198d00816..75c1beb5e70c 100644
--- a/src/lib/menu/menu-item.ts
+++ b/src/lib/menu/menu-item.ts
@@ -1,29 +1,28 @@
-import {Directive, Input, HostBinding} from '@angular/core';
+import {Directive, ElementRef, Input, HostBinding, Renderer} from '@angular/core';
 
 /**
  * This directive is intended to be used inside an md-menu tag.
  * It exists mostly to set the role attribute.
  */
 @Directive({
-  selector: 'button[md-menu-item]',
-  host: {'role': 'menuitem'}
-})
-export class MdMenuItem {}
-
-/**
- * This directive is intended to be used inside an md-menu tag.
- * It sets the role attribute and adds support for the disabled property to anchors.
- */
-@Directive({
-  selector: 'a[md-menu-item]',
+  selector: '[md-menu-item]',
   host: {
     'role': 'menuitem',
-    '(click)': 'checkDisabled($event)'
-  }
+    '(click)': '_checkDisabled($event)',
+    'tabindex': '-1'
+  },
+  exportAs: 'mdMenuItem'
 })
-export class MdMenuAnchor {
+export class MdMenuItem {
   _disabled: boolean;
 
+  constructor(private _renderer: Renderer, private _elementRef: ElementRef) {}
+
+  focus(): void {
+    this._renderer.invokeElementMethod(this._elementRef.nativeElement, 'focus');
+  }
+
+  // this is necessary to support anchors
   @HostBinding('attr.disabled')
   @Input()
   get disabled(): boolean {
@@ -39,15 +38,11 @@ export class MdMenuAnchor {
     return String(this.disabled);
   }
 
-  @HostBinding('tabIndex')
-  get tabIndex(): number {
-    return this.disabled ? -1 : 0;
-  }
-
-  checkDisabled(event: Event) {
+  private _checkDisabled(event: Event) {
     if (this.disabled) {
       event.preventDefault();
       event.stopPropagation();
     }
   }
 }
+
diff --git a/src/lib/menu/menu-trigger.ts b/src/lib/menu/menu-trigger.ts
index aec34ff0a2e3..99e7354424d3 100644
--- a/src/lib/menu/menu-trigger.ts
+++ b/src/lib/menu/menu-trigger.ts
@@ -7,7 +7,8 @@ import {
     HostListener,
     ViewContainerRef,
     AfterViewInit,
-    OnDestroy
+    OnDestroy,
+    Renderer
 } from '@angular/core';
 import {MdMenu} from './menu-directive';
 import {MdMenuMissingError} from './menu-errors';
@@ -24,6 +25,7 @@ import {
   HorizontalConnectionPos,
   VerticalConnectionPos
 } from '@angular2-material/core/overlay/position/connected-position';
+import {ENTER} from '@angular2-material/core/keyboard/keycodes';
 
 /**
  * This directive is intended to be used in conjunction with an md-menu tag.  It is
@@ -31,7 +33,10 @@ import {
  */
 @Directive({
   selector: '[md-menu-trigger-for]',
-  host: {'aria-haspopup': 'true'},
+  host: {
+    'aria-haspopup': 'true',
+    '(keydown)': '_handleKeydown($event)'
+  },
   exportAs: 'mdMenuTrigger'
 })
 export class MdMenuTrigger implements AfterViewInit, OnDestroy {
@@ -39,12 +44,16 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
   private _overlayRef: OverlayRef;
   private _menuOpen: boolean = false;
 
+  // tracking input type is necessary so it's possible to only auto-focus
+  // the first item of the list when the menu is opened via the keyboard
+  private _openedFromKeyboard: boolean = false;
+
   @Input('md-menu-trigger-for') menu: MdMenu;
   @Output() onMenuOpen = new EventEmitter();
   @Output() onMenuClose = new EventEmitter();
 
   constructor(private _overlay: Overlay, private _element: ElementRef,
-              private _viewContainerRef: ViewContainerRef) {}
+              private _viewContainerRef: ViewContainerRef, private _renderer: Renderer) {}
 
   ngAfterViewInit() {
     this._checkMenu();
@@ -63,13 +72,13 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
   openMenu(): void {
     this._createOverlay();
     this._overlayRef.attach(this._portal);
-    this._setIsMenuOpen(true);
+    this._initMenu();
   }
 
   closeMenu(): void {
     if (this._overlayRef) {
       this._overlayRef.detach();
-      this._setIsMenuOpen(false);
+      this._resetMenu();
     }
   }
 
@@ -80,6 +89,35 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
     }
   }
 
+  focus() {
+    this._renderer.invokeElementMethod(this._element.nativeElement, 'focus');
+  }
+
+  /**
+   * This method sets the menu state to open and focuses the first item if
+   * the menu was opened via the keyboard.
+   */
+  private _initMenu(): void {
+    this._setIsMenuOpen(true);
+
+    if (this._openedFromKeyboard) {
+      this.menu._focusFirstItem();
+    }
+  };
+
+  /**
+   * This method resets the menu when it's closed, most importantly restoring
+   * focus to the menu trigger if the menu was opened via the keyboard.
+   */
+  private _resetMenu(): void {
+    this._setIsMenuOpen(false);
+
+    if (this._openedFromKeyboard) {
+      this.focus();
+      this._openedFromKeyboard = false;
+    }
+  }
+
   // set state rather than toggle to support triggers sharing a menu
   private _setIsMenuOpen(isOpen: boolean): void {
     this._menuOpen = isOpen;
@@ -133,4 +171,12 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
       {overlayX: positionX, overlayY: positionY}
     );
   }
+
+  // TODO: internal
+  _handleKeydown(event: KeyboardEvent): void {
+    if (event.keyCode === ENTER) {
+      this._openedFromKeyboard = true;
+    }
+  }
+
 }
diff --git a/src/lib/menu/menu.html b/src/lib/menu/menu.html
index 33fe25d6b6b1..b6a2d1a26bd0 100644
--- a/src/lib/menu/menu.html
+++ b/src/lib/menu/menu.html
@@ -1,5 +1,5 @@
 <template>
-  <div class="md-menu" [ngClass]="_classList" (click)="_emitCloseEvent()">
+  <div class="md-menu" [ngClass]="_classList" (click)="_emitCloseEvent()" (keydown)="_handleKeydown($event)">
     <ng-content></ng-content>
   </div>
 </template>
diff --git a/src/lib/menu/menu.scss b/src/lib/menu/menu.scss
index b17b8dfa1603..c5c35b8221e5 100644
--- a/src/lib/menu/menu.scss
+++ b/src/lib/menu/menu.scss
@@ -52,7 +52,7 @@ $md-menu-vertical-padding: 8px !default;
     cursor: default;
   }
 
-  &:hover:not([disabled]) {
+  &:hover:not([disabled]), &:focus:not([disabled]) {
     background: md-color($md-background, 'hover');
   }
 }
diff --git a/src/lib/menu/menu.ts b/src/lib/menu/menu.ts
index aa841de1cd64..1394864623cc 100644
--- a/src/lib/menu/menu.ts
+++ b/src/lib/menu/menu.ts
@@ -2,17 +2,17 @@ import {NgModule, ModuleWithProviders} from '@angular/core';
 import {CommonModule} from '@angular/common';
 import {OverlayModule, OVERLAY_PROVIDERS} from '@angular2-material/core/core';
 import {MdMenu} from './menu-directive';
-import {MdMenuItem, MdMenuAnchor} from './menu-item';
+import {MdMenuItem} from './menu-item';
 import {MdMenuTrigger} from './menu-trigger';
 export {MdMenu} from './menu-directive';
-export {MdMenuItem, MdMenuAnchor} from './menu-item';
+export {MdMenuItem} from './menu-item';
 export {MdMenuTrigger} from './menu-trigger';
 
 
 @NgModule({
   imports: [OverlayModule, CommonModule],
-  exports: [MdMenu, MdMenuItem, MdMenuTrigger, MdMenuAnchor],
-  declarations: [MdMenu, MdMenuItem, MdMenuTrigger, MdMenuAnchor],
+  exports: [MdMenu, MdMenuItem, MdMenuTrigger],
+  declarations: [MdMenu, MdMenuItem, MdMenuTrigger],
 })
 export class MdMenuModule {
   static forRoot(): ModuleWithProviders {
diff --git a/src/lib/tabs/tabs.ts b/src/lib/tabs/tabs.ts
index 95415d019aea..60cd893b3b98 100644
--- a/src/lib/tabs/tabs.ts
+++ b/src/lib/tabs/tabs.ts
@@ -19,14 +19,7 @@ import {MdTabLabelWrapper} from './tab-label-wrapper';
 import {MdInkBar} from './ink-bar';
 import {Observable} from 'rxjs/Observable';
 import 'rxjs/add/operator/map';
-
-// Due to a bug in the ChromeDriver, Angular 2 keyboard events are not triggered by `sendKeys`
-// during E2E tests when using dot notation such as `(keydown.rightArrow)`. To get around this,
-// we are temporarily using a single (keydown) handler.
-// See: https://github.com/angular/angular/issues/9419
-const RIGHT_ARROW = 39;
-const LEFT_ARROW = 37;
-const ENTER = 13;
+import {RIGHT_ARROW, LEFT_ARROW, ENTER} from '@angular2-material/core/keyboard/keycodes';
 
 /** Used to generate unique ID's for each tab component */
 let nextId = 0;

From 28dbf512926a54ad02b3047cb965cfd9699801f8 Mon Sep 17 00:00:00 2001
From: Kara Erickson <karakara@google.com>
Date: Thu, 1 Sep 2016 11:12:34 -0700
Subject: [PATCH 2/3] fix readme

---
 src/lib/menu/README.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/lib/menu/README.md b/src/lib/menu/README.md
index 9ccd1c988cab..c57ed658b4e2 100644
--- a/src/lib/menu/README.md
+++ b/src/lib/menu/README.md
@@ -131,8 +131,8 @@ The menu adds `role="menu"` to the main menu element and `role="menuitem"` to ea
 also adds `aria-hasPopup="true"` to the trigger element.
 
 #### Keyboard events:
-- <kbd>DOWN_ARROW</kbd> or <kbd>TAB</kbd>: Focus next menu item
-- <kbd>UP_ARROW</kbd> or <kbd>SHIFT_TAB</kbd>: Focus previous menu item
+- <kbd>DOWN_ARROW</kbd>: Focus next menu item
+- <kbd>UP_ARROW</kbd>: Focus previous menu item
 - <kbd>ENTER</kbd>: Select focused item
 
 ### Menu attributes

From 5a8a01fd5e273055a3ddb4d0ab5572e756a2595b Mon Sep 17 00:00:00 2001
From: Kara Erickson <karakara@google.com>
Date: Thu, 1 Sep 2016 11:20:37 -0700
Subject: [PATCH 3/3] update wrap fn

---
 src/lib/menu/menu-directive.ts | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/src/lib/menu/menu-directive.ts b/src/lib/menu/menu-directive.ts
index b96dba0c2f6d..de507867ecbf 100644
--- a/src/lib/menu/menu-directive.ts
+++ b/src/lib/menu/menu-directive.ts
@@ -100,15 +100,6 @@ export class MdMenu {
     this.close.emit(null);
   }
 
-  // When focus would shift past the start or end of the menu, wrap back to the beginning or end
-  private _wrapIfFocusLeavesMenu(): void {
-    if (this._focusedItemIndex >= this.items.length) {
-      this._focusedItemIndex = 0;
-    } else if (this._focusedItemIndex < 0) {
-      this._focusedItemIndex = this.items.length - 1;
-    }
-  }
-
   private _focusNextItem(): void {
     this._updateFocusedItemIndex(1);
     this.items.toArray()[this._focusedItemIndex].focus();
@@ -130,8 +121,9 @@ export class MdMenu {
    * @private
      */
   private _updateFocusedItemIndex(delta: number, menuItems: MdMenuItem[] = this.items.toArray()) {
-    this._focusedItemIndex += delta;
-    this._wrapIfFocusLeavesMenu();
+    // when focus would leave menu, wrap to beginning or end
+    this._focusedItemIndex = (this._focusedItemIndex + delta + this.items.length)
+                              % this.items.length;
 
     // skip all disabled menu items recursively until an active one
     // is reached or the menu closes for overreaching bounds