Skip to content

Commit 25fbc95

Browse files
committed
fix(menu): not handling keyboard events when opened by mouse
Fixes the menu keyboard interactions not working when it is opened by a click. Fixes #4991.
1 parent 2f1f0fd commit 25fbc95

File tree

7 files changed

+42
-20
lines changed

7 files changed

+42
-20
lines changed

e2e/components/menu-e2e.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,9 @@ describe('menu', () => {
9797
expectFocusOn(page.items(0));
9898
});
9999

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

105105
it('should focus subsequent items when down arrow is pressed', () => {

src/lib/menu/menu-directive.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,14 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
194194
this._keyManager.setFirstItemActive();
195195
}
196196

197+
/**
198+
* Resets the active item in the menu. This is used when the menu is opened by mouse,
199+
* allowing the user to start from the first option when pressing the down arrow.
200+
*/
201+
resetActiveItem() {
202+
this._keyManager.setActiveItem(-1);
203+
}
204+
197205
/**
198206
* It's necessary to set position-based classes to ensure the menu panel animation
199207
* folds out from the correct direction.

src/lib/menu/menu-panel.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export interface MdMenuPanel {
1919
parentMenu?: MdMenuPanel | undefined;
2020
direction?: Direction;
2121
focusFirstItem: () => void;
22+
resetActiveItem: () => void;
2223
setPositionClasses: (x: MenuPositionX, y: MenuPositionY) => void;
2324
setElevation?(depth: number): void;
2425
}

src/lib/menu/menu-trigger.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,16 @@ export class MdMenuTrigger implements AfterViewInit, OnDestroy {
243243
this._setMenuElevation();
244244
this._setIsMenuOpen(true);
245245

246-
// Should only set focus if opened via the keyboard, so keyboard users can
247-
// can easily navigate menu items. According to spec, mouse users should not
248-
// see the focus style.
249-
if (!this._openedByMouse) {
246+
// If the menu was opened by mouse, we focus the root node, which allows for the keyboard
247+
// interactions to work. Otherwise, if the menu was opened by keyboard, we focus the first item.
248+
if (this._openedByMouse) {
249+
let rootNode = this._overlayRef!.overlayElement.firstElementChild as HTMLElement;
250+
251+
if (rootNode) {
252+
this.menu.resetActiveItem();
253+
rootNode.focus();
254+
}
255+
} else {
250256
this.menu.focusFirstItem();
251257
}
252258
}

src/lib/menu/menu.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
(click)="close.emit('click')"
77
[@transformMenu]="_panelAnimationState"
88
(@transformMenu.done)="_onAnimationDone($event)"
9+
tabindex="-1"
910
role="menu">
1011
<div class="mat-menu-content" [@fadeInItems]="'showing'">
1112
<ng-content></ng-content>

src/lib/menu/menu.scss

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ $mat-menu-submenu-indicator-size: 10px !default;
1515
@include mat-menu-positions();
1616
max-height: calc(100vh - #{$mat-menu-item-height});
1717
border-radius: $mat-menu-border-radius;
18+
outline: 0;
1819

1920
// Prevent the user from interacting while the panel is still animating.
2021
// This avoids issues where the user could accidentally open a sub-menu,

src/lib/menu/menu.spec.ts

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
dispatchEvent,
3131
createKeyboardEvent,
3232
createMouseEvent,
33+
dispatchFakeEvent,
3334
} from '@angular/cdk/testing';
3435

3536

@@ -174,6 +175,23 @@ describe('MdMenu', () => {
174175
expect(fixture.destroy.bind(fixture)).not.toThrow();
175176
});
176177

178+
it('should focus the menu panel root node when it was opened by mouse', () => {
179+
const fixture = TestBed.createComponent(SimpleMenu);
180+
181+
fixture.detectChanges();
182+
183+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
184+
185+
dispatchFakeEvent(triggerEl, 'mousedown');
186+
triggerEl.click();
187+
fixture.detectChanges();
188+
189+
const panel = overlayContainerElement.querySelector('.mat-menu-panel');
190+
191+
expect(panel).toBeTruthy('Expected the panel to be rendered.');
192+
expect(document.activeElement).toBe(panel, 'Expected the panel to be focused.');
193+
});
194+
177195
describe('positions', () => {
178196
let fixture: ComponentFixture<PositionedMenu>;
179197
let panel: HTMLElement;
@@ -780,20 +798,6 @@ describe('MdMenu', () => {
780798
.toBe(true, 'Expected focus to be back inside the root menu');
781799
});
782800

783-
it('should not shift focus to the sub-menu when it was opened by hover', () => {
784-
compileTestComponent();
785-
instance.rootTriggerEl.nativeElement.click();
786-
fixture.detectChanges();
787-
788-
const levelOneTrigger = overlay.querySelector('#level-one-trigger')! as HTMLElement;
789-
790-
dispatchMouseEvent(levelOneTrigger, 'mouseenter');
791-
fixture.detectChanges();
792-
793-
expect(overlay.querySelectorAll('.mat-menu-panel')[1].contains(document.activeElement))
794-
.toBe(false, 'Expected focus to not be inside the nested menu');
795-
});
796-
797801
it('should position the sub-menu to the right edge of the trigger in ltr', () => {
798802
compileTestComponent();
799803
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
@@ -1125,6 +1129,7 @@ class CustomMenuPanel implements MdMenuPanel {
11251129
@ViewChild(TemplateRef) templateRef: TemplateRef<any>;
11261130
@Output() close = new EventEmitter<void | 'click' | 'keydown'>();
11271131
focusFirstItem = () => {};
1132+
resetActiveItem = () => {};
11281133
setPositionClasses = () => {};
11291134
}
11301135

0 commit comments

Comments
 (0)