Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Sep 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions e2e/components/menu/menu-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')); }
Expand All @@ -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);
Expand Down
84 changes: 81 additions & 3 deletions e2e/components/menu/menu.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
Expand All @@ -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', () => {
Expand Down
4 changes: 2 additions & 2 deletions src/demo-app/menu/menu-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
2 changes: 2 additions & 0 deletions src/e2e-app/menu/menu-e2e.html
Original file line number Diff line number Diff line change
@@ -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">
Expand Down
13 changes: 13 additions & 0 deletions src/lib/core/keyboard/keycodes.ts
Original file line number Diff line number Diff line change
@@ -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;
9 changes: 6 additions & 3 deletions src/lib/menu/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -158,4 +162,3 @@ also adds `aria-hasPopup="true"` to the trigger element.
| `destroyMenu()` | `Promise<void>` | Destroys the menu overlay completely.



73 changes: 68 additions & 5 deletions src/lib/menu/menu-directive.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -78,8 +145,4 @@ export class MdMenu {
}
this.positionY = pos;
}

private _emitCloseEvent(): void {
this.close.emit(null);
}
}
37 changes: 16 additions & 21 deletions src/lib/menu/menu-item.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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();
}
}
}

Loading