Skip to content

Commit d822a39

Browse files
crisbetoandrewseguin
authored andcommitted
fix(menu): not handling keyboard events when opened by mouse (#4843)
Fixes the menu keyboard interactions not working when it is opened by a click. Fixes #4991.
1 parent a9600e7 commit d822a39

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
@@ -205,6 +205,14 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnDestroy {
205205
this._keyManager.setFirstItemActive();
206206
}
207207

208+
/**
209+
* Resets the active item in the menu. This is used when the menu is opened by mouse,
210+
* allowing the user to start from the first option when pressing the down arrow.
211+
*/
212+
resetActiveItem() {
213+
this._keyManager.setActiveItem(-1);
214+
}
215+
208216
/**
209217
* It's necessary to set position-based classes to ensure the menu panel animation
210218
* 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 MatMenuPanel {
1919
parentMenu?: MatMenuPanel | 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
@@ -225,10 +225,16 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
225225
this._setMenuElevation();
226226
this._setIsMenuOpen(true);
227227

228-
// Should only set focus if opened via the keyboard, so keyboard users can
229-
// can easily navigate menu items. According to spec, mouse users should not
230-
// see the focus style.
231-
if (!this._openedByMouse) {
228+
// If the menu was opened by mouse, we focus the root node, which allows for the keyboard
229+
// interactions to work. Otherwise, if the menu was opened by keyboard, we focus the first item.
230+
if (this._openedByMouse) {
231+
let rootNode = this._overlayRef!.overlayElement.firstElementChild as HTMLElement;
232+
233+
if (rootNode) {
234+
this.menu.resetActiveItem();
235+
rootNode.focus();
236+
}
237+
} else {
232238
this.menu.focusFirstItem();
233239
}
234240
}

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
@@ -14,6 +14,7 @@ $mat-menu-submenu-indicator-size: 10px !default;
1414
@include mat-menu-positions();
1515
max-height: calc(100vh - #{$mat-menu-item-height});
1616
border-radius: $mat-menu-border-radius;
17+
outline: 0;
1718

1819
// Prevent the user from interacting while the panel is still animating.
1920
// 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
@@ -33,6 +33,7 @@ import {
3333
dispatchEvent,
3434
createKeyboardEvent,
3535
createMouseEvent,
36+
dispatchFakeEvent,
3637
} from '@angular/cdk/testing';
3738

3839

@@ -191,6 +192,23 @@ describe('MatMenu', () => {
191192
expect(fixture.componentInstance.items.last.getLabel()).toBe('Item with an icon');
192193
});
193194

195+
it('should focus the menu panel root node when it was opened by mouse', () => {
196+
const fixture = TestBed.createComponent(SimpleMenu);
197+
198+
fixture.detectChanges();
199+
200+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
201+
202+
dispatchFakeEvent(triggerEl, 'mousedown');
203+
triggerEl.click();
204+
fixture.detectChanges();
205+
206+
const panel = overlayContainerElement.querySelector('.mat-menu-panel');
207+
208+
expect(panel).toBeTruthy('Expected the panel to be rendered.');
209+
expect(document.activeElement).toBe(panel, 'Expected the panel to be focused.');
210+
});
211+
194212
describe('positions', () => {
195213
let fixture: ComponentFixture<PositionedMenu>;
196214
let panel: HTMLElement;
@@ -811,20 +829,6 @@ describe('MatMenu', () => {
811829
.toBe(true, 'Expected focus to be back inside the root menu');
812830
});
813831

814-
it('should not shift focus to the sub-menu when it was opened by hover', () => {
815-
compileTestComponent();
816-
instance.rootTriggerEl.nativeElement.click();
817-
fixture.detectChanges();
818-
819-
const levelOneTrigger = overlay.querySelector('#level-one-trigger')! as HTMLElement;
820-
821-
dispatchMouseEvent(levelOneTrigger, 'mouseenter');
822-
fixture.detectChanges();
823-
824-
expect(overlay.querySelectorAll('.mat-menu-panel')[1].contains(document.activeElement))
825-
.toBe(false, 'Expected focus to not be inside the nested menu');
826-
});
827-
828832
it('should position the sub-menu to the right edge of the trigger in ltr', () => {
829833
compileTestComponent();
830834
instance.rootTriggerEl.nativeElement.style.position = 'fixed';
@@ -1179,6 +1183,7 @@ class CustomMenuPanel implements MatMenuPanel {
11791183
@ViewChild(TemplateRef) templateRef: TemplateRef<any>;
11801184
@Output() close = new EventEmitter<void | 'click' | 'keydown'>();
11811185
focusFirstItem = () => {};
1186+
resetActiveItem = () => {};
11821187
setPositionClasses = () => {};
11831188
}
11841189

0 commit comments

Comments
 (0)