Skip to content

Commit

Permalink
fix(menu): not handling keyboard events when opened by mouse
Browse files Browse the repository at this point in the history
Fixes the menu keyboard interactions not working when it is opened by a click.

Fixes #4991.
  • Loading branch information
crisbeto committed Jun 29, 2017
1 parent f73cc97 commit 6c727fe
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 8 deletions.
4 changes: 2 additions & 2 deletions e2e/components/menu-e2e.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ describe('menu', () => {
expectFocusOn(page.items(0));
});

it('should not focus the first item when opened with mouse', () => {
it('should focus the panel when opened by mouse', () => {
page.trigger().click();
expectFocusOn(page.trigger());
expectFocusOn(page.menu());
});

it('should focus subsequent items when down arrow is pressed', () => {
Expand Down
8 changes: 8 additions & 0 deletions src/lib/menu/menu-directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,14 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
this._keyManager.setFirstItemActive();
}

/**
* Resets the active item in the menu. This is used when the menu is opened by mouse,
* allowing the user to start from the first option when pressing the down arrow.
*/
resetActiveItem() {
this._keyManager.setActiveItem(-1);
}

/**
* This emits a close event to which the trigger is subscribed. When emitted, the
* trigger will close the menu.
Expand Down
1 change: 1 addition & 0 deletions src/lib/menu/menu-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface MdMenuPanel {
templateRef: TemplateRef<any>;
close: EventEmitter<void>;
focusFirstItem: () => void;
resetActiveItem: () => void;
setPositionClasses: (x: MenuPositionX, y: MenuPositionY) => void;
_emitCloseEvent: () => void;
}
14 changes: 10 additions & 4 deletions src/lib/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,10 +163,16 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
private _initMenu(): void {
this._setIsMenuOpen(true);

// Should only set focus if opened via the keyboard, so keyboard users can
// can easily navigate menu items. According to spec, mouse users should not
// see the focus style.
if (!this._openedByMouse) {
// If the menu was opened by mouse, we focus the root node, which allows for the keyboard
// interactions to work. Otherwise, if the menu was opened by keyboard, we focus the first item.
if (this._openedByMouse) {
let rootNode = this._overlayRef!.overlayElement.firstElementChild as HTMLElement;

if (rootNode) {
this.menu.resetActiveItem();
rootNode.focus();
}
} else {
this.menu.focusFirstItem();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/menu/menu.html
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<ng-template>
<div class="mat-menu-panel" [ngClass]="_classList" (keydown)="_handleKeydown($event)"
(click)="_emitCloseEvent()" [@transformMenu]="'showing'" role="menu">
(click)="_emitCloseEvent()" [@transformMenu]="'showing'" role="menu" tabindex="-1" #panel>
<div class="mat-menu-content" [@fadeInItems]="'showing'">
<ng-content></ng-content>
</div>
Expand Down
1 change: 1 addition & 0 deletions src/lib/menu/menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ $mat-menu-vertical-padding: 8px !default;
@include mat-menu-base();
@include mat-menu-positions();
max-height: calc(100vh - #{$mat-menu-item-height});
outline: 0;

@include cdk-high-contrast {
outline: solid 1px;
Expand Down
20 changes: 19 additions & 1 deletion src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {OverlayContainer} from '../core/overlay/overlay-container';
import {Directionality, Direction} from '../core/bidi/index';
import {extendObject} from '../core/util/object-extend';
import {ESCAPE} from '../core/keyboard/keycodes';
import {dispatchKeyboardEvent} from '../core/testing/dispatch-events';
import {dispatchKeyboardEvent, dispatchFakeEvent} from '../core/testing/dispatch-events';


describe('MdMenu', () => {
Expand Down Expand Up @@ -146,6 +146,23 @@ describe('MdMenu', () => {
expect(role).toBe('menu', 'Expected panel to have the "menu" role.');
});

it('should focus the menu panel root node when it was opened by mouse', () => {
const fixture = TestBed.createComponent(SimpleMenu);

fixture.detectChanges();

const triggerEl = fixture.componentInstance.triggerEl.nativeElement;

dispatchFakeEvent(triggerEl, 'mousedown');
triggerEl.click();
fixture.detectChanges();

const panel = overlayContainerElement.querySelector('.mat-menu-panel');

expect(panel).toBeTruthy('Expected the panel to be rendered.');
expect(document.activeElement).toBe(panel, 'Expected the panel to be focused.');
});

describe('positions', () => {
let fixture: ComponentFixture<PositionedMenu>;
let panel: HTMLElement;
Expand Down Expand Up @@ -553,6 +570,7 @@ class CustomMenuPanel implements MdMenuPanel {
@ViewChild(TemplateRef) templateRef: TemplateRef<any>;
@Output() close = new EventEmitter<void>();
focusFirstItem = () => {};
resetActiveItem = () => {};
setPositionClasses = () => {};
_emitCloseEvent() {
this.close.emit();
Expand Down

0 comments on commit 6c727fe

Please sign in to comment.