Skip to content

Commit bfa1853

Browse files
crisbetojelbourn
authored andcommitted
fix(menu): restore focus immediately when menu is closed (#16960)
Currently in some cases we restore focus immediately and in others we wait for the exit animation to finish (when using `matMenuContent`) because the focus restoration logic is coupled to the menu cleanup. This means that if multiple animations overlap, we could end up restoring focus to the wrong element. These changes switch to restoring focus immediately in all cases. Fixes #16954.
1 parent 306e07c commit bfa1853

File tree

3 files changed

+45
-10
lines changed

3 files changed

+45
-10
lines changed

src/material-experimental/mdc-menu/menu.spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,25 @@ describe('MatMenu', () => {
179179
expect(document.activeElement).not.toBe(triggerEl);
180180
}));
181181

182+
it('should restore focus to the trigger immediately once the menu is closed', () => {
183+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
184+
fixture.detectChanges();
185+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
186+
187+
// A click without a mousedown before it is considered a keyboard open.
188+
triggerEl.click();
189+
fixture.detectChanges();
190+
191+
expect(overlayContainerElement.querySelector('.mat-mdc-menu-panel')).toBeTruthy();
192+
193+
fixture.componentInstance.trigger.closeMenu();
194+
fixture.detectChanges();
195+
// Note: don't add a `tick` here since we're testing
196+
// that focus is restored before the animation is done.
197+
198+
expect(document.activeElement).toBe(triggerEl);
199+
});
200+
182201
it('should be able to set a custom class on the backdrop', fakeAsync(() => {
183202
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
184203

src/material/menu/menu-trigger.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -298,18 +298,20 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
298298
.subscribe({
299299
next: () => menu.lazyContent!.detach(),
300300
// No matter whether the content got re-attached, reset the menu.
301-
complete: () => this._resetMenu()
301+
complete: () => this._setIsMenuOpen(false)
302302
});
303303
} else {
304-
this._resetMenu();
304+
this._setIsMenuOpen(false);
305305
}
306306
} else {
307-
this._resetMenu();
307+
this._setIsMenuOpen(false);
308308

309309
if (menu.lazyContent) {
310310
menu.lazyContent.detach();
311311
}
312312
}
313+
314+
this._restoreFocus();
313315
}
314316

315317
/**
@@ -339,13 +341,8 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
339341
}
340342
}
341343

342-
/**
343-
* This method resets the menu when it's closed, most importantly restoring
344-
* focus to the menu trigger if the menu was opened via the keyboard.
345-
*/
346-
private _resetMenu(): void {
347-
this._setIsMenuOpen(false);
348-
344+
/** Restores focus to the element that was focused before the menu was open. */
345+
private _restoreFocus() {
349346
// We should reset focus if the user is navigating using a keyboard or
350347
// if we have a top-level trigger which might cause focus to be lost
351348
// when clicking on the backdrop.

src/material/menu/menu.spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,25 @@ describe('MatMenu', () => {
181181
expect(document.activeElement).not.toBe(triggerEl);
182182
}));
183183

184+
it('should restore focus to the trigger immediately once the menu is closed', () => {
185+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
186+
fixture.detectChanges();
187+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
188+
189+
// A click without a mousedown before it is considered a keyboard open.
190+
triggerEl.click();
191+
fixture.detectChanges();
192+
193+
expect(overlayContainerElement.querySelector('.mat-menu-panel')).toBeTruthy();
194+
195+
fixture.componentInstance.trigger.closeMenu();
196+
fixture.detectChanges();
197+
// Note: don't add a `tick` here since we're testing
198+
// that focus is restored before the animation is done.
199+
200+
expect(document.activeElement).toBe(triggerEl);
201+
});
202+
184203
it('should be able to set a custom class on the backdrop', fakeAsync(() => {
185204
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
186205

0 commit comments

Comments
 (0)