diff --git a/src/lib/menu/menu-directive.ts b/src/lib/menu/menu-directive.ts index 78700a5584ee..eff65ef2c290 100644 --- a/src/lib/menu/menu-directive.ts +++ b/src/lib/menu/menu-directive.ts @@ -40,7 +40,7 @@ import { OnInit, } from '@angular/core'; import {merge, Observable, Subject, Subscription} from 'rxjs'; -import {startWith, switchMap, take} from 'rxjs/operators'; +import {startWith, switchMap, take, debounceTime} from 'rxjs/operators'; import {matMenuAnimations} from './menu-animations'; import {MatMenuContent} from './menu-content'; import {throwMatMenuInvalidPositionX, throwMatMenuInvalidPositionY} from './menu-errors'; @@ -259,11 +259,34 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI ngAfterContentInit() { this._keyManager = new FocusKeyManager(this._items).withWrap().withTypeAhead(); this._tabSubscription = this._keyManager.tabOut.subscribe(() => this.closed.emit('tab')); + + // TODO(crisbeto): the `debounce` here should be removed since it's something + // that people have to flush in their tests. It'll be possible once we switch back + // to using a QueryList in #11720. + this._ngZone.runOutsideAngular(() => { + // 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._itemChanges.pipe(debounceTime(50)).subscribe(items => { + const manager = this._keyManager; + + 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() { this._tabSubscription.unsubscribe(); this.closed.complete(); + this._itemChanges.complete(); } /** Stream that emits whenever the hovered menu item changes. */ @@ -374,7 +397,7 @@ export class MatMenu implements AfterContentInit, MatMenuPanel, OnI removeItem(item: MatMenuItem) { const index = this._items.indexOf(item); - if (this._items.indexOf(item) > -1) { + if (index > -1) { this._items.splice(index, 1); this._itemChanges.next(this._items); } diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index a48d104baa6c..127e4b5ff2c0 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -180,6 +180,26 @@ describe('MatMenu', () => { expect(document.activeElement).not.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]); @@ -237,6 +257,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'); @@ -2299,7 +2320,6 @@ class DynamicPanelMenu { @ViewChild('two') secondMenu: MatMenu; } - @Component({ template: ` @@ -2313,3 +2333,19 @@ class DynamicPanelMenu { class MenuWithCheckboxItems { @ViewChild(MatMenuTrigger) trigger: MatMenuTrigger; } + + +@Component({ + template: ` + + + + + ` +}) +class MenuWithRepeatedItems { + @ViewChild(MatMenuTrigger) trigger: MatMenuTrigger; + @ViewChild('triggerEl') triggerEl: ElementRef; + @ViewChild(MatMenu) menu: MatMenu; + items = ['One', 'Two', 'Three']; +}