Skip to content

Commit 2ad7df4

Browse files
crisbetojosephperrott
authored andcommitted
fix(menu): proper focus styling when opened by tap on a touch device (#13599)
1 parent 53bdbd5 commit 2ad7df4

File tree

2 files changed

+62
-7
lines changed

2 files changed

+62
-7
lines changed

src/lib/menu/menu-trigger.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export const MENU_PANEL_TOP_PADDING = 8;
7272
'aria-haspopup': 'true',
7373
'[attr.aria-expanded]': 'menuOpen || null',
7474
'(mousedown)': '_handleMousedown($event)',
75+
'(touchstart)': '_openedBy = "touch"',
7576
'(keydown)': '_handleKeydown($event)',
7677
'(click)': '_handleClick($event)',
7778
},
@@ -87,7 +88,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
8788

8889
// Tracking input type is necessary so it's possible to only auto-focus
8990
// the first item of the list when the menu is opened via the keyboard
90-
private _openedByMouse: boolean = false;
91+
_openedBy: 'mouse' | 'touch' | null = null;
9192

9293
/**
9394
* @deprecated
@@ -281,7 +282,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
281282
this.menu.direction = this.dir;
282283
this._setMenuElevation();
283284
this._setIsMenuOpen(true);
284-
this.menu.focusFirstItem(this._openedByMouse ? 'mouse' : 'program');
285+
this.menu.focusFirstItem(this._openedBy || 'program');
285286
}
286287

287288
/** Updates the menu elevation based on the amount of parent menus that it has. */
@@ -309,15 +310,15 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
309310
// We should reset focus if the user is navigating using a keyboard or
310311
// if we have a top-level trigger which might cause focus to be lost
311312
// when clicking on the backdrop.
312-
if (!this._openedByMouse) {
313+
if (!this._openedBy) {
313314
// Note that the focus style will show up both for `program` and
314315
// `keyboard` so we don't have to specify which one it is.
315316
this.focus();
316317
} else if (!this.triggersSubmenu()) {
317-
this.focus('mouse');
318+
this.focus(this._openedBy);
318319
}
319320

320-
this._openedByMouse = false;
321+
this._openedBy = null;
321322
}
322323

323324
// set state rather than toggle to support triggers sharing a menu
@@ -464,7 +465,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
464465
if (!isFakeMousedownFromScreenReader(event)) {
465466
// Since right or middle button clicks won't trigger the `click` event,
466467
// we shouldn't consider the menu as opened by mouse in those cases.
467-
this._openedByMouse = event.button === 0;
468+
this._openedBy = event.button === 0 ? 'mouse' : null;
468469

469470
// Since clicking on the trigger won't close the menu if it opens a sub-menu,
470471
// we should prevent focus from moving onto it via click to avoid the
@@ -513,7 +514,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
513514
delay(0, asapScheduler)
514515
)
515516
.subscribe(() => {
516-
this._openedByMouse = true;
517+
this._openedBy = 'mouse';
517518

518519
// If the same menu is used between multiple triggers, it might still be animating
519520
// while the new trigger tries to re-open it. Wait for the animation to finish

src/lib/menu/menu.spec.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,24 @@ describe('MatMenu', () => {
164164
expect(document.activeElement).toBe(triggerEl);
165165
}));
166166

167+
it('should restore focus to the root trigger when the menu was opened by touch', fakeAsync(() => {
168+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
169+
fixture.detectChanges();
170+
171+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
172+
dispatchFakeEvent(triggerEl, 'touchstart');
173+
triggerEl.click();
174+
fixture.detectChanges();
175+
176+
expect(overlayContainerElement.querySelector('.mat-menu-panel')).toBeTruthy();
177+
178+
fixture.componentInstance.trigger.closeMenu();
179+
fixture.detectChanges();
180+
flush();
181+
182+
expect(document.activeElement).toBe(triggerEl);
183+
}));
184+
167185
it('should scroll the panel to the top on open, when it is scrollable', fakeAsync(() => {
168186
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
169187
fixture.detectChanges();
@@ -247,6 +265,27 @@ describe('MatMenu', () => {
247265
focusMonitor.stopMonitoring(triggerEl);
248266
}));
249267

268+
it('should set the proper focus origin when restoring focus after opening by touch',
269+
fakeAsync(() => {
270+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
271+
fixture.detectChanges();
272+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
273+
274+
dispatchMouseEvent(triggerEl, 'touchstart');
275+
triggerEl.click();
276+
fixture.detectChanges();
277+
patchElementFocus(triggerEl);
278+
focusMonitor.monitor(triggerEl, false);
279+
fixture.componentInstance.trigger.closeMenu();
280+
fixture.detectChanges();
281+
tick(500);
282+
fixture.detectChanges();
283+
flush();
284+
285+
expect(triggerEl.classList).toContain('cdk-touch-focused');
286+
focusMonitor.stopMonitoring(triggerEl);
287+
}));
288+
250289
it('should close the menu when pressing ESCAPE', fakeAsync(() => {
251290
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
252291
fixture.detectChanges();
@@ -378,6 +417,21 @@ describe('MatMenu', () => {
378417
expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledWith('mouse');
379418
}));
380419

420+
it('should set the proper focus origin when opening by touch', fakeAsync(() => {
421+
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
422+
fixture.detectChanges();
423+
spyOn(fixture.componentInstance.items.first, 'focus').and.callThrough();
424+
425+
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
426+
427+
dispatchMouseEvent(triggerEl, 'touchstart');
428+
triggerEl.click();
429+
fixture.detectChanges();
430+
flush();
431+
432+
expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledWith('touch');
433+
}));
434+
381435
it('should close the menu when using the CloseScrollStrategy', fakeAsync(() => {
382436
const scrolledSubject = new Subject();
383437
const fixture = createComponent(SimpleMenu, [

0 commit comments

Comments
 (0)