Skip to content

Commit 26590c0

Browse files
committed
fix(material/menu): moving focus on destroy not working without animations module
We have some logic that will move focus in the menu if the active item is destroyed. This seems to only work when there's an animations module available, because otherwise the item is removed from the DOM by the time we check if it's focused. These changes resolve the issue by basing the logic on the state in the key manager.
1 parent 4de4b29 commit 26590c0

File tree

4 files changed

+5
-14
lines changed

4 files changed

+5
-14
lines changed

goldens/material/menu/index.api.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,6 @@ export class MatMenuItem implements FocusableOption, AfterViewInit, OnDestroy {
195195
getLabel(): string;
196196
_getTabIndex(): string;
197197
_handleMouseEnter(): void;
198-
// (undocumented)
199-
_hasFocus(): boolean;
200198
_highlighted: boolean;
201199
readonly _hovered: Subject<MatMenuItem>;
202200
// (undocumented)

src/material/menu/menu-item.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import {
1717
ChangeDetectorRef,
1818
booleanAttribute,
1919
inject,
20-
DOCUMENT,
2120
} from '@angular/core';
2221
import {FocusableOption, FocusMonitor, FocusOrigin} from '@angular/cdk/a11y';
2322
import {Subject} from 'rxjs';
@@ -50,7 +49,6 @@ import {_CdkPrivateStyleLoader} from '@angular/cdk/private';
5049
})
5150
export class MatMenuItem implements FocusableOption, AfterViewInit, OnDestroy {
5251
private _elementRef = inject<ElementRef<HTMLElement>>(ElementRef);
53-
private _document = inject(DOCUMENT);
5452
private _focusMonitor = inject(FocusMonitor);
5553
_parentMenu? = inject<MatMenuPanel<MatMenuItem>>(MAT_MENU_PANEL, {optional: true});
5654
private _changeDetectorRef = inject(ChangeDetectorRef);
@@ -164,8 +162,4 @@ export class MatMenuItem implements FocusableOption, AfterViewInit, OnDestroy {
164162
this._triggersSubmenu = triggersSubmenu;
165163
this._changeDetectorRef.markForCheck();
166164
}
167-
168-
_hasFocus(): boolean {
169-
return this._document && this._document.activeElement === this._getHostElement();
170-
}
171165
}

src/material/menu/menu.spec.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import {
3030
} from '@angular/core';
3131
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
3232
import {By} from '@angular/platform-browser';
33-
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
3433
import {Subject} from 'rxjs';
3534
import {
3635
createKeyboardEvent,
@@ -256,10 +255,6 @@ describe('MatMenu', () => {
256255
}));
257256

258257
it('should move focus to another item if the active item is destroyed', fakeAsync(() => {
259-
// TODO(crisbeto): figure out why NoopAnimationsModule is necessary
260-
// here and our token isn't enough. Likely indicates an issue.
261-
TestBed.resetTestingModule().configureTestingModule({imports: [NoopAnimationsModule]});
262-
overlayContainerElement = TestBed.inject(OverlayContainer).getContainerElement();
263258
const fixture = TestBed.createComponent(MenuWithRepeatedItems);
264259
fixture.detectChanges();
265260
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;

src/material/menu/menu.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,11 @@ export class MatMenu implements AfterContentInit, MatMenuPanel<MatMenuItem>, OnI
320320
// in quick succession.
321321
const manager = this._keyManager;
322322

323-
if (this._panelAnimationState === 'enter' && manager.activeItem?._hasFocus()) {
323+
if (
324+
this._panelAnimationState === 'enter' &&
325+
manager.activeItem &&
326+
!itemsList.some(item => item === manager.activeItem)
327+
) {
324328
const items = itemsList.toArray();
325329
const index = Math.max(0, Math.min(items.length - 1, manager.activeItemIndex || 0));
326330

0 commit comments

Comments
 (0)