Skip to content

fix(menu): proper focus styling when opened by tap on a touch device #13599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions src/lib/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const MENU_PANEL_TOP_PADDING = 8;
'aria-haspopup': 'true',
'[attr.aria-expanded]': 'menuOpen || null',
'(mousedown)': '_handleMousedown($event)',
'(touchstart)': '_openedBy = "touch"',
'(keydown)': '_handleKeydown($event)',
'(click)': '_handleClick($event)',
},
Expand All @@ -87,7 +88,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {

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

/**
* @deprecated
Expand Down Expand Up @@ -281,7 +282,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
this.menu.direction = this.dir;
this._setMenuElevation();
this._setIsMenuOpen(true);
this.menu.focusFirstItem(this._openedByMouse ? 'mouse' : 'program');
this.menu.focusFirstItem(this._openedBy || 'program');
}

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

this._openedByMouse = false;
this._openedBy = null;
}

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

// Since clicking on the trigger won't close the menu if it opens a sub-menu,
// we should prevent focus from moving onto it via click to avoid the
Expand Down Expand Up @@ -508,7 +509,7 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
delay(0, asapScheduler)
)
.subscribe(() => {
this._openedByMouse = true;
this._openedBy = 'mouse';

// If the same menu is used between multiple triggers, it might still be animating
// while the new trigger tries to re-open it. Wait for the animation to finish
Expand Down
54 changes: 54 additions & 0 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,24 @@ describe('MatMenu', () => {
expect(document.activeElement).toBe(triggerEl);
}));

it('should restore focus to the root trigger when the menu was opened by touch', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();

const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
dispatchFakeEvent(triggerEl, 'touchstart');
triggerEl.click();
fixture.detectChanges();

expect(overlayContainerElement.querySelector('.mat-menu-panel')).toBeTruthy();

fixture.componentInstance.trigger.closeMenu();
fixture.detectChanges();
flush();

expect(document.activeElement).toBe(triggerEl);
}));

it('should scroll the panel to the top on open, when it is scrollable', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
Expand Down Expand Up @@ -247,6 +265,27 @@ describe('MatMenu', () => {
focusMonitor.stopMonitoring(triggerEl);
}));

it('should set the proper focus origin when restoring focus after opening by touch',
fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;

dispatchMouseEvent(triggerEl, 'touchstart');
triggerEl.click();
fixture.detectChanges();
patchElementFocus(triggerEl);
focusMonitor.monitor(triggerEl, false);
fixture.componentInstance.trigger.closeMenu();
fixture.detectChanges();
tick(500);
fixture.detectChanges();
flush();

expect(triggerEl.classList).toContain('cdk-touch-focused');
focusMonitor.stopMonitoring(triggerEl);
}));

it('should close the menu when pressing ESCAPE', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
Expand Down Expand Up @@ -380,6 +419,21 @@ describe('MatMenu', () => {
expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledWith('mouse');
}));

it('should set the proper focus origin when opening by touch', fakeAsync(() => {
const fixture = createComponent(SimpleMenu, [], [FakeIcon]);
fixture.detectChanges();
spyOn(fixture.componentInstance.items.first, 'focus').and.callThrough();

const triggerEl = fixture.componentInstance.triggerEl.nativeElement;

dispatchMouseEvent(triggerEl, 'touchstart');
triggerEl.click();
fixture.detectChanges();
flush();

expect(fixture.componentInstance.items.first.focus).toHaveBeenCalledWith('touch');
}));

it('should close the menu when using the CloseScrollStrategy', fakeAsync(() => {
const scrolledSubject = new Subject();
const fixture = createComponent(SimpleMenu, [
Expand Down