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..f9e892d5b4f8 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,137 @@ 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 focus subsequent items when tab is pressed', () => { + page.pressKey(protractor.Key.ENTER); + page.pressKey(protractor.Key.TAB); + page.expectFocusOn(page.items(1)); + }); + + it('should focus previous items when shift-tab is pressed', () => { + page.pressKey(protractor.Key.ENTER); + page.pressKey(protractor.Key.TAB); + // need a protractor "chord" to hit shift-tab simultaneously + page.pressKey(protractor.Key.chord(protractor.Key.SHIFT, protractor.Key.TAB)); + page.expectFocusOn(page.items(0)); + }); + + it('should handle a mix of tabs and arrow presses', () => { + page.pressKey(protractor.Key.ENTER); + page.pressKey(protractor.Key.TAB); + page.pressKey(protractor.Key.UP); + page.expectFocusOn(page.items(0)); + + page.pressKey(protractor.Key.DOWN); + page.pressKey(protractor.Key.chord(protractor.Key.SHIFT, protractor.Key.TAB)); + 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 skip disabled items using tabs', () => { + page.pressKey(protractor.Key.ENTER); + page.pressKey(protractor.Key.TAB); + page.pressKey(protractor.Key.TAB); + page.expectFocusOn(page.items(3)); + + page.pressKey(protractor.Key.chord(protractor.Key.SHIFT, protractor.Key.TAB)); + 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.pressKey(protractor.Key.TAB); + 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 close the 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.expectMenuPresent(false); + + page.start().click(); + page.pressKey(protractor.Key.TAB); + page.pressKey(protractor.Key.ENTER); + page.pressKey(protractor.Key.UP); + page.expectMenuPresent(false); + }); + + it('should focus before and after trigger when tabbing past items', () => { + page.pressKey(protractor.Key.ENTER); + page.pressKey(protractor.Key.TAB); + page.pressKey(protractor.Key.TAB); + 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()); + }); + + it('should focus on trigger 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.trigger()); + + page.pressKey(protractor.Key.ENTER); + page.pressKey(protractor.Key.UP); + page.expectFocusOn(page.trigger()); + }); + }); + describe('position - ', () => { it('should default menu alignment to "after below" when not set', () => { diff --git a/src/components/menu/README.md b/src/components/menu/README.md index 35d098269563..3e9242acfce5 100644 --- a/src/components/menu/README.md +++ b/src/components/menu/README.md @@ -129,7 +129,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: +- DOWN_ARROW or TAB: Focus next menu item +- UP_ARROW or SHIFT_TAB: Focus previous menu item +- ENTER: Select focused item ### Menu attributes @@ -160,7 +165,6 @@ also adds `aria-hasPopup="true"` to the trigger element. ### TODO -- Keyboard events: up arrow, down arrow, enter - `prevent-close` option, to turn off automatic menu close when clicking outside the menu - Custom offset support diff --git a/src/components/menu/menu-directive.ts b/src/components/menu/menu-directive.ts index bac6f9181a1c..91028c763b73 100644 --- a/src/components/menu/menu-directive.ts +++ b/src/components/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 { private _showClickCatcher: boolean = false; + private _focusedItemIndex: number = 0; // config object to be passed into the menu's ngClass private _classList: Object; @@ -33,6 +37,7 @@ export class MdMenu { positionY: MenuPositionY = 'below'; @ViewChild(TemplateRef) templateRef: TemplateRef; + @ContentChildren(MdMenuItem) items: QueryList; constructor(@Attribute('x-position') posX: MenuPositionX, @Attribute('y-position') posY: MenuPositionY) { @@ -65,6 +70,72 @@ 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._handleTabKeypress(event.shiftKey); + } + } + + /** + * When the tab key is pressed (changing focus natively), this function syncs up + * the focusedItemIndex to match the currently focused item. If the shift key is + * also pressed, we know that the focus has changed to the previous tabindex. If not, + * focus has changed to the next tabindex. + */ + private _handleTabKeypress(shiftPressed: boolean): void { + shiftPressed ? this._updateFocusedItemIndex(-1) : this._updateFocusedItemIndex(1); + } + + /** + * 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, close the menu. + private _closeIfFocusLeavesMenu(): void { + if (this._focusedItemIndex >= this.items.length || this._focusedItemIndex < 0) { + this._emitCloseEvent(); + } + } + + private _focusNextItem(): void { + this._updateFocusedItemIndex(1); + this.items.toArray()[this._focusedItemIndex].focus(); + } + + private _focusPreviousItem(): void { + this._updateFocusedItemIndex(-1); + this.items.toArray()[this._focusedItemIndex].focus(); + } + + private _updateFocusedItemIndex(delta: number) { + this._focusedItemIndex += delta; + this._closeIfFocusLeavesMenu(); + + // skip all disabled menu items recursively until an active one + // is reached or the menu closes for overreaching bounds + while (this.items.toArray()[this._focusedItemIndex].disabled) { + this._updateFocusedItemIndex(delta); + } + } + private _setPositionX(pos: MenuPositionX): void { if ( pos !== 'before' && pos !== 'after') { throw new MdMenuInvalidPositionX(); @@ -78,8 +149,4 @@ export class MdMenu { } this.positionY = pos; } - - private _emitCloseEvent(): void { - this.close.emit(null); - } } diff --git a/src/components/menu/menu-item.ts b/src/components/menu/menu-item.ts index 28b198d00816..3195dec25fd3 100644 --- a/src/components/menu/menu-item.ts +++ b/src/components/menu/menu-item.ts @@ -1,29 +1,27 @@ -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)' + }, + 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 { @@ -38,16 +36,16 @@ export class MdMenuAnchor { get isAriaDisabled(): string { 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/components/menu/menu-trigger.ts b/src/components/menu/menu-trigger.ts index 0ed91cc974ca..45eb46ef7d67 100644 --- a/src/components/menu/menu-trigger.ts +++ b/src/components/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'; @@ -25,6 +26,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 @@ -32,7 +34,10 @@ import { */ @Directive({ selector: '[md-menu-trigger-for]', - host: {'aria-haspopup': 'true'}, + host: { + 'aria-haspopup': 'true', + '(keydown)': '_handleKeydown($event)' + }, providers: [OVERLAY_PROVIDERS], exportAs: 'mdMenuTrigger' }) @@ -40,13 +45,14 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { private _portal: TemplatePortal; private _overlayRef: OverlayRef; private _menuOpen: boolean = false; + 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(); @@ -65,14 +71,14 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { openMenu(): Promise { return this._createOverlay() .then(() => this._overlayRef.attach(this._portal)) - .then(() => this._setIsMenuOpen(true)); + .then(() => this._prepMenu()); } closeMenu(): Promise { if (!this._overlayRef) { return Promise.resolve(); } return this._overlayRef.detach() - .then(() => this._setIsMenuOpen(false)); + .then(() => this._resetMenu()); } destroyMenu(): void { @@ -82,6 +88,27 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy { } } + focus() { + this._renderer.invokeElementMethod(this._element.nativeElement, 'focus'); + } + + private _prepMenu(): void { + this._setIsMenuOpen(true); + + if (this._openedFromKeyboard) { + this.menu._focusFirstItem(); + } + }; + + 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; @@ -136,4 +163,10 @@ 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/components/menu/menu.html b/src/components/menu/menu.html index 33fe25d6b6b1..b6a2d1a26bd0 100644 --- a/src/components/menu/menu.html +++ b/src/components/menu/menu.html @@ -1,5 +1,5 @@ diff --git a/src/components/menu/menu.scss b/src/components/menu/menu.scss index b17b8dfa1603..c5c35b8221e5 100644 --- a/src/components/menu/menu.scss +++ b/src/components/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/components/menu/menu.ts b/src/components/menu/menu.ts index bdd37c428986..e47f8cc95970 100644 --- a/src/components/menu/menu.ts +++ b/src/components/menu/menu.ts @@ -1,9 +1,9 @@ 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'; -export const MD_MENU_DIRECTIVES = [MdMenu, MdMenuItem, MdMenuTrigger, MdMenuAnchor]; +export const MD_MENU_DIRECTIVES = [MdMenu, MdMenuItem, MdMenuTrigger]; diff --git a/src/components/tabs/tabs.ts b/src/components/tabs/tabs.ts index 7c7772107490..8e2414593614 100644 --- a/src/components/tabs/tabs.ts +++ b/src/components/tabs/tabs.ts @@ -18,14 +18,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; diff --git a/src/core/keyboard/keycodes.ts b/src/core/keyboard/keycodes.ts new file mode 100644 index 000000000000..df5a0aae4c12 --- /dev/null +++ b/src/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/demo-app/menu/menu-demo.ts b/src/demo-app/menu/menu-demo.ts index 2d0e04a3d610..aaf3ad0690fb 100644 --- a/src/demo-app/menu/menu-demo.ts +++ b/src/demo-app/menu/menu-demo.ts @@ -21,8 +21,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 @@
{{ selected }}
+ @@ -8,6 +9,7 @@ +