feat(menu): add keyboard events and improve accessibility (#1132)

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);

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', () => {

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; }

src/e2e-app/menu/menu-e2e.html

@@ -1,13 +1,15 @@
 <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>
 
     <md-menu #menu="mdMenu" class="custom">
       <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">

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;

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>: Focus next menu item
+- <kbd>UP_ARROW</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. 
 
 
-

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,68 @@ 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);
+  }
+
+  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()) {
+    // 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
+    while (menuItems[this._focusedItemIndex].disabled) {
+      this._updateFocusedItemIndex(delta, menuItems);
+    }
+  }
+
   private _setPositionX(pos: MenuPositionX): void {
     if ( pos !== 'before' && pos !== 'after') {
       throw new MdMenuInvalidPositionX();
@@ -78,8 +145,4 @@ export class MdMenu {
     }
     this.positionY = pos;
   }
-
-  private _emitCloseEvent(): void {
-    this.close.emit(null);
-  }
 }

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();
     }
   }
 }
+