Skip to content

Commit 48842ca

Browse files
committed
fix(menu): focus lost if active item is removed
Fixes the user's focus being lost if the active item is destroyed while a menu is open. Fixes #14028.
1 parent 7c75d6e commit 48842ca

File tree

2 files changed

+54
-1
lines changed

2 files changed

+54
-1
lines changed

src/material/menu/menu.spec.ts

+36-1
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,26 @@ describe('MatMenu', () => {
231231
expect(document.activeElement).toBe(triggerEl);
232232
});
233233

234+
it('should move focus to another item if the active item is destroyed', fakeAsync(() => {
235+
const fixture = createComponent(MenuWithRepeatedItems, [], [FakeIcon]);
236+
fixture.detectChanges();
237+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
238+
239+
triggerEl.click();
240+
fixture.detectChanges();
241+
tick(500);
242+
243+
const items = overlayContainerElement.querySelectorAll('.mat-menu-panel .mat-menu-item');
244+
245+
expect(document.activeElement).toBe(items[0]);
246+
247+
fixture.componentInstance.items.shift();
248+
fixture.detectChanges();
249+
tick(500);
250+
251+
expect(document.activeElement).toBe(items[1]);
252+
}));
253+
234254
it('should be able to set a custom class on the backdrop', fakeAsync(() => {
235255
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
236256

@@ -288,6 +308,7 @@ describe('MatMenu', () => {
288308
// Add 50 items to make the menu scrollable
289309
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
290310
fixture.detectChanges();
311+
tick(50);
291312

292313
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
293314
dispatchFakeEvent(triggerEl, 'mousedown');
@@ -2515,7 +2536,6 @@ class DynamicPanelMenu {
25152536
@ViewChild('two') secondMenu: MatMenu;
25162537
}
25172538

2518-
25192539
@Component({
25202540
template: `
25212541
<button [matMenuTriggerFor]="menu">Toggle menu</button>
@@ -2590,3 +2610,18 @@ class LazyMenuWithOnPush {
25902610
@ViewChild('triggerEl', {read: ElementRef}) rootTrigger: ElementRef;
25912611
@ViewChild('menuItem', {read: ElementRef}) menuItemWithSubmenu: ElementRef;
25922612
}
2613+
2614+
@Component({
2615+
template: `
2616+
<button [matMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
2617+
<mat-menu #menu="matMenu">
2618+
<button *ngFor="let item of items" mat-menu-item>{{item}}</button>
2619+
</mat-menu>
2620+
`
2621+
})
2622+
class MenuWithRepeatedItems {
2623+
@ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
2624+
@ViewChild('triggerEl', {static: false}) triggerEl: ElementRef<HTMLElement>;
2625+
@ViewChild(MatMenu, {static: false}) menu: MatMenu;
2626+
items = ['One', 'Two', 'Three'];
2627+
}

src/material/menu/menu.ts

+18
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,24 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
268268
startWith(this._directDescendantItems),
269269
switchMap(items => merge<MatMenuItem>(...items.map((item: MatMenuItem) => item._focused)))
270270
).subscribe(focusedItem => this._keyManager.updateActiveItem(focusedItem));
271+
272+
// Move focus to another item, if the active item is removed from the list.
273+
// We need to debounce the callback, because multiple items might be removed
274+
// in quick succession.
275+
this._directDescendantItems.changes.subscribe(itemsList => {
276+
const manager = this._keyManager;
277+
const items = itemsList.toArray();
278+
279+
if (manager.activeItem && items.indexOf(manager.activeItem) === -1) {
280+
const index = Math.max(0, Math.min(items.length - 1, manager.activeItemIndex || 0));
281+
282+
if (items[index] && !items[index].disabled) {
283+
manager.setActiveItem(index);
284+
} else {
285+
manager.setNextItemActive();
286+
}
287+
}
288+
});
271289
}
272290

273291
ngOnDestroy() {

0 commit comments

Comments
 (0)