Skip to content

Commit 757e482

Browse files
committed
fix(menu): always focus first menu item
* Switches to always focusing the first menu item, even when opening by mouse. The focus styling will only apply when focused by keyboard. * Adds the ability to set the focus origin through the `FocusKeyManager`. Fixes #9252.
1 parent 60b0625 commit 757e482

File tree

9 files changed

+79
-35
lines changed

9 files changed

+79
-35
lines changed

src/cdk/a11y/focus-key-manager.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88

99
import {ListKeyManager, ListKeyManagerOption} from './list-key-manager';
10+
import {FocusOrigin} from './focus-monitor';
1011

1112
/**
1213
* This is the interface for focusable items (used by the FocusKeyManager).
@@ -15,10 +16,21 @@ import {ListKeyManager, ListKeyManagerOption} from './list-key-manager';
1516
*/
1617
export interface FocusableOption extends ListKeyManagerOption {
1718
/** Focuses the `FocusableOption`. */
18-
focus(): void;
19+
focus(origin?: FocusOrigin): void;
1920
}
2021

2122
export class FocusKeyManager<T> extends ListKeyManager<FocusableOption & T> {
23+
private _origin: FocusOrigin = 'program';
24+
25+
/**
26+
* Sets the focus origin that will be passed in to the items for any subsequent `focus` calls.
27+
* @param origin Focus origin to be used when focusing items.
28+
*/
29+
setFocusOrigin(origin: FocusOrigin): this {
30+
this._origin = origin;
31+
return this;
32+
}
33+
2234
/**
2335
* This method sets the active item to the item at the specified index.
2436
* It also adds focuses the newly active item.
@@ -27,7 +39,7 @@ export class FocusKeyManager<T> extends ListKeyManager<FocusableOption & T> {
2739
super.setActiveItem(index);
2840

2941
if (this.activeItem) {
30-
this.activeItem.focus();
42+
this.activeItem.focus(this._origin);
3143
}
3244
}
3345
}

src/cdk/a11y/list-key-manager.spec.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@ import {createKeyboardEvent} from '../testing/event-objects';
66
import {ActiveDescendantKeyManager} from './activedescendant-key-manager';
77
import {FocusKeyManager} from './focus-key-manager';
88
import {ListKeyManager} from './list-key-manager';
9+
import {FocusOrigin} from './focus-monitor';
910

1011

1112
class FakeFocusable {
1213
constructor(private _label = '') { }
1314
disabled = false;
14-
focus() {}
15+
focus(_focusOrigin?: FocusOrigin) {}
1516
getLabel() { return this._label; }
1617
}
1718

@@ -626,6 +627,21 @@ describe('Key managers', () => {
626627
expect(itemList.items[1].focus).not.toHaveBeenCalledTimes(1);
627628
});
628629

630+
it('should be able to set the focus origin', () => {
631+
keyManager.setFocusOrigin('mouse');
632+
633+
keyManager.onKeydown(fakeKeyEvents.downArrow);
634+
expect(itemList.items[1].focus).toHaveBeenCalledWith('mouse');
635+
636+
keyManager.onKeydown(fakeKeyEvents.downArrow);
637+
expect(itemList.items[2].focus).toHaveBeenCalledWith('mouse');
638+
639+
keyManager.setFocusOrigin('keyboard');
640+
641+
keyManager.onKeydown(fakeKeyEvents.upArrow);
642+
expect(itemList.items[1].focus).toHaveBeenCalledWith('keyboard');
643+
});
644+
629645
});
630646

631647
describe('ActiveDescendantKeyManager', () => {

src/lib/menu/_menu-theme.scss

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@
2626
}
2727

2828
.mat-menu-item:hover,
29-
.mat-menu-item:focus,
29+
.mat-menu-item.cdk-program-focused,
30+
.mat-menu-item.cdk-keyboard-focused,
3031
.mat-menu-item-highlighted {
3132
&:not([disabled]) {
3233
background: mat-color($background, 'hover');

src/lib/menu/menu-directive.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import {MatMenuItem} from './menu-item';
4040
import {MatMenuPanel} from './menu-panel';
4141
import {MenuPositionX, MenuPositionY} from './menu-positions';
4242
import {coerceBooleanProperty} from '@angular/cdk/coercion';
43+
import {FocusOrigin} from '@angular/cdk/a11y';
4344

4445

4546
/** Default `mat-menu` options that can be overridden. */
@@ -228,16 +229,17 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnDestroy {
228229
}
229230

230231
/**
231-
* Focus the first item in the menu. This method is used by the menu trigger
232-
* to focus the first item when the menu is opened by the ENTER key.
232+
* Focus the first item in the menu.
233+
* @param origin Action from which the focus originated. Used to set the correct styling.
233234
*/
234-
focusFirstItem() {
235-
this._keyManager.setFirstItemActive();
235+
focusFirstItem(origin?: FocusOrigin) {
236+
// TODO(crisbeto): make the origin required when doing breaking changes.
237+
this._keyManager.setFocusOrigin(origin).setFirstItemActive();
236238
}
237239

238240
/**
239-
* Resets the active item in the menu. This is used when the menu is opened by mouse,
240-
* allowing the user to start from the first option when pressing the down arrow.
241+
* Resets the active item in the menu. This is used when the menu is opened, allowing
242+
* the user to start from the first option when pressing the down arrow.
241243
*/
242244
resetActiveItem() {
243245
this._keyManager.setActiveItem(-1);

src/lib/menu/menu-item.ts

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {FocusableOption} from '@angular/cdk/a11y';
9+
import {FocusableOption, FocusMonitor, FocusOrigin} from '@angular/cdk/a11y';
1010
import {
1111
ChangeDetectionStrategy,
1212
Component,
@@ -64,16 +64,34 @@ export class MatMenuItem extends _MatMenuItemMixinBase
6464
/** Whether the menu item acts as a trigger for a sub-menu. */
6565
_triggersSubmenu: boolean = false;
6666

67-
constructor(private _elementRef: ElementRef) {
67+
constructor(
68+
private _elementRef: ElementRef,
69+
// TODO(crisbeto): switch to a required param when doing breaking changes.
70+
private _focusMonitor?: FocusMonitor) {
6871
super();
72+
73+
if (_focusMonitor) {
74+
// Start monitoring the element so it gets the appropriate focused classes. We want
75+
// to show the focus style for menu items only when the focus was not caused by a
76+
// mouse or touch interaction.
77+
_focusMonitor.monitor(this._getHostElement(), false);
78+
}
6979
}
7080

7181
/** Focuses the menu item. */
72-
focus(): void {
73-
this._getHostElement().focus();
82+
focus(origin: FocusOrigin = 'program'): void {
83+
if (this._focusMonitor) {
84+
this._focusMonitor.focusVia(this._getHostElement(), origin);
85+
} else {
86+
this._getHostElement().focus();
87+
}
7488
}
7589

7690
ngOnDestroy() {
91+
if (this._focusMonitor) {
92+
this._focusMonitor.stopMonitoring(this._getHostElement());
93+
}
94+
7795
this._hovered.complete();
7896
}
7997

src/lib/menu/menu-module.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {MatMenu, MAT_MENU_DEFAULT_OPTIONS} from './menu-directive';
1414
import {MatMenuItem} from './menu-item';
1515
import {MatMenuTrigger, MAT_MENU_SCROLL_STRATEGY_PROVIDER} from './menu-trigger';
1616
import {MatRippleModule} from '@angular/material/core';
17+
import {A11yModule} from '@angular/cdk/a11y';
1718

1819

1920
@NgModule({
@@ -22,6 +23,7 @@ import {MatRippleModule} from '@angular/material/core';
2223
CommonModule,
2324
MatRippleModule,
2425
MatCommonModule,
26+
A11yModule,
2527
],
2628
exports: [MatMenu, MatMenuItem, MatMenuTrigger, MatCommonModule],
2729
declarations: [MatMenu, MatMenuItem, MatMenuTrigger],

src/lib/menu/menu-panel.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import {EventEmitter, TemplateRef} from '@angular/core';
1010
import {MenuPositionX, MenuPositionY} from './menu-positions';
1111
import {Direction} from '@angular/cdk/bidi';
12+
import {FocusOrigin} from '@angular/cdk/a11y';
1213

1314
/**
1415
* Interface for a custom menu panel that can be used with `matMenuTriggerFor`.
@@ -22,7 +23,7 @@ export interface MatMenuPanel {
2223
close: EventEmitter<void | 'click' | 'keydown'>;
2324
parentMenu?: MatMenuPanel | undefined;
2425
direction?: Direction;
25-
focusFirstItem: () => void;
26+
focusFirstItem: (origin?: FocusOrigin) => void;
2627
resetActiveItem: () => void;
2728
setPositionClasses: (x: MenuPositionX, y: MenuPositionY) => void;
2829
setElevation?(depth: number): void;

src/lib/menu/menu-trigger.ts

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -234,19 +234,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
234234
this.menu.direction = this.dir;
235235
this._setMenuElevation();
236236
this._setIsMenuOpen(true);
237-
238-
// If the menu was opened by mouse, we focus the root node, which allows for the keyboard
239-
// interactions to work. Otherwise, if the menu was opened by keyboard, we focus the first item.
240-
if (this._openedByMouse) {
241-
let rootNode = this._overlayRef!.overlayElement.firstElementChild as HTMLElement;
242-
243-
if (rootNode) {
244-
this.menu.resetActiveItem();
245-
rootNode.focus();
246-
}
247-
} else {
248-
this.menu.focusFirstItem();
249-
}
237+
this.menu.focusFirstItem(this._openedByMouse ? 'mouse' : 'program');
250238
}
251239

252240
/** Updates the menu elevation based on the amount of parent menus that it has. */

src/lib/menu/menu.spec.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -230,22 +230,20 @@ describe('MatMenu', () => {
230230
expect(fixture.componentInstance.items.last.getLabel()).toBe('Item with an icon');
231231
});
232232

233-
it('should focus the menu panel root node when it was opened by mouse', () => {
233+
it('should set the proper focus origin when opening by mouse', fakeAsync(() => {
234234
const fixture = TestBed.createComponent(SimpleMenu);
235-
236235
fixture.detectChanges();
236+
spyOn(fixture.componentInstance.items.first, 'focus').and.callThrough();
237237

238238
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
239239

240240
dispatchFakeEvent(triggerEl, 'mousedown');
241241
triggerEl.click();
242242
fixture.detectChanges();
243+
tick(500);
243244

244-
const panel = overlayContainerElement.querySelector('.mat-menu-panel')!;
245-
246-
expect(panel).toBeTruthy('Expected the panel to be rendered.');
247-
expect(document.activeElement).toBe(panel, 'Expected the panel to be focused.');
248-
});
245+
expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledWith('mouse');
246+
}));
249247

250248
it('should close the menu when using the CloseScrollStrategy', fakeAsync(() => {
251249
const scrolledSubject = new Subject();
@@ -1068,6 +1066,7 @@ describe('MatMenu', () => {
10681066

10691067
instance.alternateTrigger.openMenu();
10701068
fixture.detectChanges();
1069+
tick(500);
10711070

10721071
lastMenu = overlay.querySelector('.mat-menu-panel') as HTMLElement;
10731072

@@ -1121,6 +1120,7 @@ describe('MatMenu', () => {
11211120
compileTestComponent();
11221121
instance.rootTriggerEl.nativeElement.click();
11231122
fixture.detectChanges();
1123+
tick(500);
11241124
expect(overlay.querySelectorAll('.mat-menu-panel').length).toBe(1, 'Expected one open menu');
11251125

11261126
instance.showLazy = true;
@@ -1130,6 +1130,8 @@ describe('MatMenu', () => {
11301130

11311131
dispatchMouseEvent(lazyTrigger, 'mouseenter');
11321132
fixture.detectChanges();
1133+
tick(500);
1134+
11331135
expect(lazyTrigger.classList)
11341136
.toContain('mat-menu-item-highlighted', 'Expected the trigger to be highlighted');
11351137
expect(overlay.querySelectorAll('.mat-menu-panel').length).toBe(2, 'Expected two open menus');
@@ -1157,10 +1159,12 @@ describe('MatMenu', () => {
11571159

11581160
repeaterFixture.componentInstance.rootTriggerEl.nativeElement.click();
11591161
repeaterFixture.detectChanges();
1162+
tick(500);
11601163
expect(overlay.querySelectorAll('.mat-menu-panel').length).toBe(1, 'Expected one open menu');
11611164

11621165
dispatchMouseEvent(overlay.querySelector('.level-one-trigger')!, 'mouseenter');
11631166
repeaterFixture.detectChanges();
1167+
tick(500);
11641168
expect(overlay.querySelectorAll('.mat-menu-panel').length).toBe(2, 'Expected two open menus');
11651169
}));
11661170

0 commit comments

Comments
 (0)