Skip to content

Commit

Permalink
fix(menu): set appropriate origin when restoring focus (#9303)
Browse files Browse the repository at this point in the history
Sets the correct focus origin depending on the way a menu has been opened.

Fixes #9292.
  • Loading branch information
crisbeto authored and jelbourn committed Jan 23, 2018
1 parent 8972bf4 commit 278e25a
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 17 deletions.
18 changes: 6 additions & 12 deletions src/cdk/a11y/focus-monitor.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import {TAB} from '@angular/cdk/keycodes';
import {dispatchFakeEvent, dispatchKeyboardEvent, dispatchMouseEvent} from '@angular/cdk/testing';
import {
dispatchFakeEvent,
dispatchKeyboardEvent,
dispatchMouseEvent,
patchElementFocus,
} from '@angular/cdk/testing';
import {Component} from '@angular/core';
import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
Expand Down Expand Up @@ -413,14 +418,3 @@ class ComplexComponentWithMonitorElementFocus {}
template: `<div tabindex="0" cdkMonitorSubtreeFocus><button></button></div>`
})
class ComplexComponentWithMonitorSubtreeFocus {}


/**
* Patches an elements focus and blur methods to emit events consistently and predictably.
* This is necessary, because some browsers, like IE11, will call the focus handlers asynchronously,
* while others won't fire them at all if the browser window is not focused.
*/
function patchElementFocus(element: HTMLElement) {
element.focus = () => dispatchFakeEvent(element, 'focus');
element.blur = () => dispatchFakeEvent(element, 'blur');
}
19 changes: 19 additions & 0 deletions src/cdk/testing/element-focus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {dispatchFakeEvent} from './dispatch-events';

/**
* Patches an elements focus and blur methods to emit events consistently and predictably.
* This is necessary, because some browsers, like IE11, will call the focus handlers asynchronously,
* while others won't fire them at all if the browser window is not focused.
*/
export function patchElementFocus(element: HTMLElement) {
element.focus = () => dispatchFakeEvent(element, 'focus');
element.blur = () => dispatchFakeEvent(element, 'blur');
}
1 change: 1 addition & 0 deletions src/cdk/testing/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export * from './event-objects';
export * from './type-in-element';
export * from './wrapped-error-message';
export * from './mock-ng-zone';
export * from './element-focus';
2 changes: 2 additions & 0 deletions src/lib/menu/menu-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {NgModule} from '@angular/core';
import {CommonModule} from '@angular/common';
import {MatCommonModule} from '@angular/material/core';
import {OverlayModule} from '@angular/cdk/overlay';
import {A11yModule} from '@angular/cdk/a11y';
import {MatMenu, MAT_MENU_DEFAULT_OPTIONS} from './menu-directive';
import {MatMenuItem} from './menu-item';
import {MatMenuTrigger, MAT_MENU_SCROLL_STRATEGY_PROVIDER} from './menu-trigger';
Expand All @@ -21,6 +22,7 @@ import {A11yModule} from '@angular/cdk/a11y';
imports: [
OverlayModule,
CommonModule,
A11yModule,
MatRippleModule,
MatCommonModule,
A11yModule,
Expand Down
24 changes: 19 additions & 5 deletions src/lib/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {throwMatMenuMissingError} from './menu-errors';
import {MatMenuItem} from './menu-item';
import {MatMenuPanel} from './menu-panel';
import {MenuPositionX, MenuPositionY} from './menu-positions';
import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y';

/** Injection token that determines the scroll handling while the menu is open. */
export const MAT_MENU_SCROLL_STRATEGY =
Expand Down Expand Up @@ -130,7 +131,9 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
@Inject(MAT_MENU_SCROLL_STRATEGY) private _scrollStrategy,
@Optional() private _parentMenu: MatMenu,
@Optional() @Self() private _menuItemInstance: MatMenuItem,
@Optional() private _dir: Directionality) {
@Optional() private _dir: Directionality,
// TODO(crisbeto): make the _focusMonitor required when doing breaking changes.
private _focusMonitor?: FocusMonitor) {

if (_menuItemInstance) {
_menuItemInstance._triggersSubmenu = this.triggersSubmenu();
Expand Down Expand Up @@ -207,9 +210,16 @@ export class MatMenuTrigger implements AfterContentInit, OnDestroy {
this.menu.close.emit();
}

/** Focuses the menu trigger. */
focus() {
this._element.nativeElement.focus();
/**
* Focuses the menu trigger.
* @param origin Source of the menu trigger's focus.
*/
focus(origin: FocusOrigin = 'program') {
if (this._focusMonitor) {
this._focusMonitor.focusVia(this._element.nativeElement, origin);
} else {
this._element.nativeElement.focus();
}
}

/** Closes the menu and does the necessary cleanup. */
Expand Down Expand Up @@ -262,8 +272,12 @@ 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 || !this.triggersSubmenu()) {
if (!this._openedByMouse) {
// 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._openedByMouse = false;
Expand Down
41 changes: 41 additions & 0 deletions src/lib/menu/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,11 @@ import {
createKeyboardEvent,
createMouseEvent,
dispatchFakeEvent,
patchElementFocus,
} from '@angular/cdk/testing';
import {Subject} from 'rxjs/Subject';
import {ScrollDispatcher} from '@angular/cdk/scrolling';
import {FocusMonitor} from '@angular/cdk/a11y';


describe('MatMenu', () => {
Expand Down Expand Up @@ -142,6 +144,45 @@ describe('MatMenu', () => {
expect(document.activeElement).toBe(triggerEl);
}));

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

patchElementFocus(triggerEl);
focusMonitor.monitor(triggerEl, false);
triggerEl.click(); // A click without a mousedown before it is considered a keyboard open.
fixture.detectChanges();
fixture.componentInstance.trigger.closeMenu();
fixture.detectChanges();
tick(500);
fixture.detectChanges();

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

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

dispatchFakeEvent(triggerEl, 'mousedown');
triggerEl.click();
fixture.detectChanges();
patchElementFocus(triggerEl);
focusMonitor.monitor(triggerEl, false);
fixture.componentInstance.trigger.closeMenu();
fixture.detectChanges();
tick(500);
fixture.detectChanges();

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

it('should close the menu when pressing ESCAPE', fakeAsync(() => {
const fixture = TestBed.createComponent(SimpleMenu);
fixture.detectChanges();
Expand Down

0 comments on commit 278e25a

Please sign in to comment.