Skip to content

Commit c0efe14

Browse files
authored
fix(material/menu): custom origin in focusFirstItem not respected on open (#25812)
A while ago we added an `NgZone.onStable` wrapped around the logic that moves focus in `MatMenu.focusFirstItem` in order to work around an issue with VoiceOver. Since focus is now delayed, calling `focusFirstItem` in quick succession with different origins may not work as expected. These changes unsubscribe from the previous `onStable` callback if a new one comes in before it has finished. Fixes #25658.
1 parent 502faa7 commit c0efe14

File tree

3 files changed

+57
-2
lines changed

3 files changed

+57
-2
lines changed

src/material/legacy-menu/menu.spec.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,32 @@ describe('MatMenu', () => {
768768
expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledWith('touch');
769769
}));
770770

771+
it('should set the proper origin when calling focusFirstItem after the opening sequence has started', () => {
772+
let zone: MockNgZone;
773+
const fixture = createComponent(
774+
SimpleMenu,
775+
[
776+
{
777+
provide: NgZone,
778+
useFactory: () => (zone = new MockNgZone()),
779+
},
780+
],
781+
[FakeIcon],
782+
);
783+
fixture.detectChanges();
784+
spyOn(fixture.componentInstance.items.first, 'focus').and.callThrough();
785+
786+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
787+
788+
dispatchMouseEvent(triggerEl, 'mousedown');
789+
triggerEl.click();
790+
fixture.detectChanges();
791+
fixture.componentInstance.menu.focusFirstItem('touch');
792+
zone!.onStable.next();
793+
794+
expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledOnceWith('touch');
795+
});
796+
771797
it('should close the menu when using the CloseScrollStrategy', fakeAsync(() => {
772798
const scrolledSubject = new Subject();
773799
const fixture = createComponent(

src/material/menu/menu.spec.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,32 @@ describe('MDC-based MatMenu', () => {
771771
expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledWith('touch');
772772
}));
773773

774+
it('should set the proper origin when calling focusFirstItem after the opening sequence has started', () => {
775+
let zone: MockNgZone;
776+
const fixture = createComponent(
777+
SimpleMenu,
778+
[
779+
{
780+
provide: NgZone,
781+
useFactory: () => (zone = new MockNgZone()),
782+
},
783+
],
784+
[FakeIcon],
785+
);
786+
fixture.detectChanges();
787+
spyOn(fixture.componentInstance.items.first, 'focus').and.callThrough();
788+
789+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
790+
791+
dispatchMouseEvent(triggerEl, 'mousedown');
792+
triggerEl.click();
793+
fixture.detectChanges();
794+
fixture.componentInstance.menu.focusFirstItem('touch');
795+
zone!.onStable.next();
796+
797+
expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledOnceWith('touch');
798+
});
799+
774800
it('should close the menu when using the CloseScrollStrategy', fakeAsync(() => {
775801
const scrolledSubject = new Subject();
776802
const fixture = createComponent(

src/material/menu/menu.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import {
4040
UP_ARROW,
4141
hasModifierKey,
4242
} from '@angular/cdk/keycodes';
43-
import {merge, Observable, Subject} from 'rxjs';
43+
import {merge, Observable, Subject, Subscription} from 'rxjs';
4444
import {startWith, switchMap, take} from 'rxjs/operators';
4545
import {MatMenuItem} from './menu-item';
4646
import {MatMenuPanel, MAT_MENU_PANEL} from './menu-panel';
@@ -102,6 +102,7 @@ export class _MatMenuBase
102102
private _keyManager: FocusKeyManager<MatMenuItem>;
103103
private _xPosition: MenuPositionX = this._defaultOptions.xPosition;
104104
private _yPosition: MenuPositionY = this._defaultOptions.yPosition;
105+
private _firstItemFocusSubscription?: Subscription;
105106
private _previousElevation: string;
106107
protected _elevationPrefix: string;
107108
protected _baseElevation: number;
@@ -337,6 +338,7 @@ export class _MatMenuBase
337338
this._keyManager?.destroy();
338339
this._directDescendantItems.destroy();
339340
this.closed.complete();
341+
this._firstItemFocusSubscription?.unsubscribe();
340342
}
341343

342344
/** Stream that emits whenever the hovered menu item changes. */
@@ -407,7 +409,8 @@ export class _MatMenuBase
407409
*/
408410
focusFirstItem(origin: FocusOrigin = 'program'): void {
409411
// Wait for `onStable` to ensure iOS VoiceOver screen reader focuses the first item (#24735).
410-
this._ngZone.onStable.pipe(take(1)).subscribe(() => {
412+
this._firstItemFocusSubscription?.unsubscribe();
413+
this._firstItemFocusSubscription = this._ngZone.onStable.pipe(take(1)).subscribe(() => {
411414
let menuPanel: HTMLElement | null = null;
412415

413416
if (this._directDescendantItems.length) {

0 commit comments

Comments
 (0)