Skip to content

Commit

Permalink
fix(menu): focus lost if active item is removed
Browse files Browse the repository at this point in the history
Fixes the user's focus being lost if the active item is destroyed while a menu is open.

Fixes #14028.
  • Loading branch information
crisbeto committed Mar 20, 2020
1 parent 7c75d6e commit 48842ca
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 1 deletion.
37 changes: 36 additions & 1 deletion src/material/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,26 @@ describe('MatMenu', () => {
expect(document.activeElement).toBe(triggerEl);
});

it('should move focus to another item if the active item is destroyed', fakeAsync(() => {
const fixture = createComponent(MenuWithRepeatedItems, [], [FakeIcon]);
fixture.detectChanges();
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;

triggerEl.click();
fixture.detectChanges();
tick(500);

const items = overlayContainerElement.querySelectorAll('.mat-menu-panel .mat-menu-item');

expect(document.activeElement).toBe(items[0]);

fixture.componentInstance.items.shift();
fixture.detectChanges();
tick(500);

expect(document.activeElement).toBe(items[1]);
}));

it('should be able to set a custom class on the backdrop', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);

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

const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
dispatchFakeEvent(triggerEl, 'mousedown');
Expand Down Expand Up @@ -2515,7 +2536,6 @@ class DynamicPanelMenu {
@ViewChild('two') secondMenu: MatMenu;
}


@Component({
template: `
<button [matMenuTriggerFor]="menu">Toggle menu</button>
Expand Down Expand Up @@ -2590,3 +2610,18 @@ class LazyMenuWithOnPush {
@ViewChild('triggerEl', {read: ElementRef}) rootTrigger: ElementRef;
@ViewChild('menuItem', {read: ElementRef}) menuItemWithSubmenu: ElementRef;
}

@Component({
template: `
<button [matMenuTriggerFor]="menu" #triggerEl>Toggle menu</button>
<mat-menu #menu="matMenu">
<button *ngFor="let item of items" mat-menu-item>{{item}}</button>
</mat-menu>
`
})
class MenuWithRepeatedItems {
@ViewChild(MatMenuTrigger, {static: false}) trigger: MatMenuTrigger;
@ViewChild('triggerEl', {static: false}) triggerEl: ElementRef<HTMLElement>;
@ViewChild(MatMenu, {static: false}) menu: MatMenu;
items = ['One', 'Two', 'Three'];
}
18 changes: 18 additions & 0 deletions src/material/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,24 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
startWith(this._directDescendantItems),
switchMap(items => merge<MatMenuItem>(...items.map((item: MatMenuItem) => item._focused)))
).subscribe(focusedItem => this._keyManager.updateActiveItem(focusedItem));

// Move focus to another item, if the active item is removed from the list.
// We need to debounce the callback, because multiple items might be removed
// in quick succession.
this._directDescendantItems.changes.subscribe(itemsList => {
const manager = this._keyManager;
const items = itemsList.toArray();

if (manager.activeItem && items.indexOf(manager.activeItem) === -1) {
const index = Math.max(0, Math.min(items.length - 1, manager.activeItemIndex || 0));

if (items[index] && !items[index].disabled) {
manager.setActiveItem(index);
} else {
manager.setNextItemActive();
}
}
});
}

ngOnDestroy() {
Expand Down

0 comments on commit 48842ca

Please sign in to comment.