From 3bc82f6dc82ebe2f489f3ef3655a4f33f2e8b8fe Mon Sep 17 00:00:00 2001 From: mmalerba Date: Wed, 7 Jun 2017 14:58:17 -0700 Subject: [PATCH] fix(sidenav): don't restore focus if focus isn't inside sidenav (#4578) * fix(sidenav): don't restore focus if focus isn't inside sidenav * inject document --- src/lib/sidenav/sidenav.spec.ts | 48 ++++++++++++++++++++++++++------- src/lib/sidenav/sidenav.ts | 26 +++++++++++++----- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/src/lib/sidenav/sidenav.spec.ts b/src/lib/sidenav/sidenav.spec.ts index 9ed30addcae..e787b001aaf 100644 --- a/src/lib/sidenav/sidenav.spec.ts +++ b/src/lib/sidenav/sidenav.spec.ts @@ -1,5 +1,5 @@ import {fakeAsync, async, tick, ComponentFixture, TestBed} from '@angular/core/testing'; -import {Component, ViewChild} from '@angular/core'; +import {Component, ElementRef, ViewChild} from '@angular/core'; import {By} from '@angular/platform-browser'; import {MdSidenav, MdSidenavModule, MdSidenavToggleResult, MdSidenavContainer} from './index'; import {A11yModule} from '../core/a11y/index'; @@ -310,19 +310,20 @@ describe('MdSidenav', () => { expect(testComponent.closeCount).toBe(0); })); - it('should restore focus to the trigger element on close', fakeAsync(() => { + it('should restore focus on close if focus is inside sidenav', fakeAsync(() => { let fixture = TestBed.createComponent(BasicTestApp); let sidenav: MdSidenav = fixture.debugElement .query(By.directive(MdSidenav)).componentInstance; - let trigger = document.createElement('button'); + let openButton = fixture.componentInstance.openButton.nativeElement; + let sidenavButton = fixture.componentInstance.sidenavButton.nativeElement; - document.body.appendChild(trigger); - trigger.focus(); + openButton.focus(); sidenav.open(); fixture.detectChanges(); endSidenavTransition(fixture); tick(); + sidenavButton.focus(); sidenav.close(); @@ -331,9 +332,32 @@ describe('MdSidenav', () => { tick(); expect(document.activeElement) - .toBe(trigger, 'Expected focus to be restored to the trigger on close.'); + .toBe(openButton, 'Expected focus to be restored to the open button on close.'); + })); - trigger.parentNode.removeChild(trigger); + it('should not restore focus on close if focus is outside sidenav', fakeAsync(() => { + let fixture = TestBed.createComponent(BasicTestApp); + let sidenav: MdSidenav = fixture.debugElement + .query(By.directive(MdSidenav)).componentInstance; + let openButton = fixture.componentInstance.openButton.nativeElement; + let closeButton = fixture.componentInstance.closeButton.nativeElement; + + openButton.focus(); + sidenav.open(); + + fixture.detectChanges(); + endSidenavTransition(fixture); + tick(); + closeButton.focus(); + + sidenav.close(); + + fixture.detectChanges(); + endSidenavTransition(fixture); + tick(); + + expect(document.activeElement) + .toBe(closeButton, 'Expected focus not to be restored to the open button on close.'); })); }); @@ -508,10 +532,10 @@ class SidenavContainerTwoSidenavTestApp { (open)="open()" (close-start)="closeStart()" (close)="close()"> - Content. + - - + + `, }) class BasicTestApp { @@ -521,6 +545,10 @@ class BasicTestApp { closeCount: number = 0; backdropClickedCount: number = 0; + @ViewChild('sidenavButton') sidenavButton: ElementRef; + @ViewChild('openButton') openButton: ElementRef; + @ViewChild('closeButton') closeButton: ElementRef; + openStart() { this.openStartCount++; } diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts index 437eb0cffa4..f6d31da5707 100644 --- a/src/lib/sidenav/sidenav.ts +++ b/src/lib/sidenav/sidenav.ts @@ -12,12 +12,13 @@ import { Renderer2, ViewEncapsulation, NgZone, - OnDestroy, + OnDestroy, Inject, } from '@angular/core'; import {Dir, coerceBooleanProperty} from '../core'; import {FocusTrapFactory, FocusTrap} from '../core/a11y/focus-trap'; import {ESCAPE} from '../core/keyboard/keycodes'; import 'rxjs/add/operator/first'; +import {DOCUMENT} from '@angular/platform-browser'; /** Throws an exception when two MdSidenav are matching the same side. */ @@ -126,24 +127,37 @@ export class MdSidenav implements AfterContentInit, OnDestroy { * @param _elementRef The DOM element reference. Used for transition and width calculation. * If not available we do not hook on transitions. */ - constructor(private _elementRef: ElementRef, private _focusTrapFactory: FocusTrapFactory) { + constructor(private _elementRef: ElementRef, + private _focusTrapFactory: FocusTrapFactory, + @Optional() @Inject(DOCUMENT) private _doc: any) { this.onOpen.subscribe(() => { - this._elementFocusedBeforeSidenavWasOpened = document.activeElement as HTMLElement; + if (this._doc) { + this._elementFocusedBeforeSidenavWasOpened = this._doc.activeElement as HTMLElement; + } if (this.isFocusTrapEnabled && this._focusTrap) { this._focusTrap.focusInitialElementWhenReady(); } }); - this.onClose.subscribe(() => { + this.onClose.subscribe(() => this._restoreFocus()); + } + + /** + * If focus is currently inside the sidenav, restores it to where it was before the sidenav + * opened. + */ + private _restoreFocus() { + let activeEl = this._doc && this._doc.activeElement; + if (activeEl && this._elementRef.nativeElement.contains(activeEl)) { if (this._elementFocusedBeforeSidenavWasOpened instanceof HTMLElement) { this._elementFocusedBeforeSidenavWasOpened.focus(); } else { this._elementRef.nativeElement.blur(); } + } - this._elementFocusedBeforeSidenavWasOpened = null; - }); + this._elementFocusedBeforeSidenavWasOpened = null; } ngAfterContentInit() {