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 Aug 15, 2017
1 parent 85a6fff commit 408a05f
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 20 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 @@ -96,9 +96,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 @@ -193,6 +193,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);
}

/**
* It's necessary to set position-based classes to ensure the menu panel animation
* folds out from the correct direction.
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 @@ -19,6 +19,7 @@ export interface MdMenuPanel {
parentMenu?: MdMenuPanel | undefined;
direction?: Direction;
focusFirstItem: () => void;
resetActiveItem: () => void;
setPositionClasses: (x: MenuPositionX, y: MenuPositionY) => void;
setElevation?(depth: number): 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 @@ -239,10 +239,16 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
this._setMenuElevation();
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
1 change: 1 addition & 0 deletions src/lib/menu/menu.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
(click)="close.emit('click')"
[@transformMenu]="_panelAnimationState"
(@transformMenu.done)="_onAnimationDone($event)"
tabindex="-1"
role="menu">
<div class="mat-menu-content" [@fadeInItems]="'showing'">
<ng-content></ng-content>
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 @@ -14,6 +14,7 @@ $mat-menu-submenu-indicator-size: 10px !default;
@include mat-menu-positions();
max-height: calc(100vh - #{$mat-menu-item-height});
border-radius: $mat-menu-border-radius;
outline: 0;

// Prevent the user from interacting while the panel is still animating.
// This avoids issues where the user could accidentally open a sub-menu,
Expand Down
33 changes: 19 additions & 14 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
dispatchMouseEvent,
dispatchEvent,
createKeyboardEvent,
dispatchFakeEvent,
} from '@angular/cdk/testing';


Expand Down Expand Up @@ -173,6 +174,23 @@ describe('MdMenu', () => {
expect(fixture.destroy.bind(fixture)).not.toThrow();
});

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 @@ -770,20 +788,6 @@ describe('MdMenu', () => {
.toBe(true, 'Expected focus to be back inside the root menu');
});

it('should not shift focus to the sub-menu when it was opened by hover', () => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.click();
fixture.detectChanges();

const levelOneTrigger = overlay.querySelector('#level-one-trigger')! as HTMLElement;

dispatchMouseEvent(levelOneTrigger, 'mouseenter');
fixture.detectChanges();

expect(overlay.querySelectorAll('.mat-menu-panel')[1].contains(document.activeElement))
.toBe(false, 'Expected focus to not be inside the nested menu');
});

it('should position the sub-menu to the right edge of the trigger in ltr', () => {
compileTestComponent();
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
Expand Down Expand Up @@ -1057,6 +1061,7 @@ class CustomMenuPanel implements MdMenuPanel {
@ViewChild(TemplateRef) templateRef: TemplateRef<any>;
@Output() close = new EventEmitter<void | 'click' | 'keydown'>();
focusFirstItem = () => {};
resetActiveItem = () => {};
setPositionClasses = () => {};
}

Expand Down

0 comments on commit 408a05f

Please sign in to comment.