Skip to content

Commit ed99a0a

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 ec4ea06 commit ed99a0a

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
@@ -193,6 +193,14 @@ export class MdMenu implements AfterContentInit, MdMenuPanel, OnDestroy {
193193
this._keyManager.setFirstItemActive();
194194
}
195195

196+
/**
197+
* Resets the active item in the menu. This is used when the menu is opened by mouse,
198+
* allowing the user to start from the first option when pressing the down arrow.
199+
*/
200+
resetActiveItem() {
201+
this._keyManager.setActiveItem(-1);
202+
}
203+
196204
/**
197205
* It's necessary to set position-based classes to ensure the menu panel animation
198206
* 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
@@ -29,6 +29,7 @@ import {
2929
dispatchMouseEvent,
3030
dispatchEvent,
3131
createKeyboardEvent,
32+
dispatchFakeEvent,
3233
} from '@angular/cdk/testing';
3334

3435

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

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

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

0 commit comments

Comments
 (0)