From 45a14503af744d106e448632eb450e8da6d54aac Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 18 Feb 2017 23:52:17 +0100 Subject: [PATCH] refactor(focus-trap): convert to directive Refactors the focus trap to be used as a directive, rather than a component. This gives us a couple of advantages: * It can be used on the same node as other components. * It removes a level of nesting in the DOM. This makes it slightly more convenient to style projected in cases like the dialog (see #2546), where flexbox needs to be applied to the closest possible ancestor. Also includes the following improvements: * No longer triggers change detection when focus hits the start/end anchors. * Resets the anchor tab index when trapping is disabled, instead of removing elements from the DOM. * Adds missing unit tests for the disabled and cleanup logic. --- src/lib/core/a11y/focus-trap.html | 3 -- src/lib/core/a11y/focus-trap.spec.ts | 53 +++++++++++++++------ src/lib/core/a11y/focus-trap.ts | 70 ++++++++++++++++++++-------- src/lib/sidenav/sidenav.scss | 13 ++---- 4 files changed, 95 insertions(+), 44 deletions(-) delete mode 100644 src/lib/core/a11y/focus-trap.html diff --git a/src/lib/core/a11y/focus-trap.html b/src/lib/core/a11y/focus-trap.html deleted file mode 100644 index 5950764b3851..000000000000 --- a/src/lib/core/a11y/focus-trap.html +++ /dev/null @@ -1,3 +0,0 @@ -
-
-
diff --git a/src/lib/core/a11y/focus-trap.spec.ts b/src/lib/core/a11y/focus-trap.spec.ts index 37889b2f226c..f28bf172809d 100644 --- a/src/lib/core/a11y/focus-trap.spec.ts +++ b/src/lib/core/a11y/focus-trap.spec.ts @@ -1,6 +1,5 @@ -import {inject, ComponentFixture, TestBed, async} from '@angular/core/testing'; -import {By} from '@angular/platform-browser'; -import {Component} from '@angular/core'; +import {ComponentFixture, TestBed, async} from '@angular/core/testing'; +import {Component, ViewChild} from '@angular/core'; import {FocusTrap} from './focus-trap'; import {InteractivityChecker} from './interactivity-checker'; import {Platform} from '../platform/platform'; @@ -21,16 +20,15 @@ describe('FocusTrap', () => { }); TestBed.compileComponents(); - })); - beforeEach(inject([InteractivityChecker], (c: InteractivityChecker) => { fixture = TestBed.createComponent(FocusTrapTestApp); - focusTrapInstance = fixture.debugElement.query(By.directive(FocusTrap)).componentInstance; + fixture.detectChanges(); + focusTrapInstance = fixture.componentInstance.focusTrap; })); it('wrap focus from end to start', () => { // Because we can't mimic a real tab press focus change in a unit test, just call the - // focus event handler directly. + // focus event handlerdiv directly. focusTrapInstance.focusFirstTabbableElement(); expect(document.activeElement.nodeName.toLowerCase()) @@ -48,6 +46,30 @@ describe('FocusTrap', () => { expect(document.activeElement.nodeName.toLowerCase()) .toBe(lastElement, `Expected ${lastElement} element to be focused`); }); + + it('should clean up its anchor sibling elements on destroy', () => { + const rootElement = fixture.debugElement.nativeElement as HTMLElement; + + expect(rootElement.querySelectorAll('div.cdk-visually-hidden').length).toBe(2); + + fixture.componentInstance.renderFocusTrap = false; + fixture.detectChanges(); + + expect(rootElement.querySelectorAll('div.cdk-visually-hidden').length).toBe(0); + }); + + it('should set the appropriate tabindex on the anchors, based on the disabled state', () => { + const anchors = Array.from( + fixture.debugElement.nativeElement.querySelectorAll('div.cdk-visually-hidden') + ) as HTMLElement[]; + + expect(anchors.every(current => current.getAttribute('tabindex') === '0')).toBe(true); + + fixture.componentInstance.isFocusTrapDisabled = true; + fixture.detectChanges(); + + expect(anchors.every(current => current.getAttribute('tabindex') === '-1')).toBe(true); + }); }); describe('with focus targets', () => { @@ -61,11 +83,10 @@ describe('FocusTrap', () => { }); TestBed.compileComponents(); - })); - beforeEach(inject([InteractivityChecker], (c: InteractivityChecker) => { fixture = TestBed.createComponent(FocusTrapTargetTestApp); - focusTrapInstance = fixture.debugElement.query(By.directive(FocusTrap)).componentInstance; + fixture.detectChanges(); + focusTrapInstance = fixture.componentInstance.focusTrap; })); it('should be able to prioritize the first focus target', () => { @@ -87,13 +108,17 @@ describe('FocusTrap', () => { @Component({ template: ` - + ` }) -class FocusTrapTestApp { } +class FocusTrapTestApp { + @ViewChild(FocusTrap) focusTrap: FocusTrap; + renderFocusTrap = true; + isFocusTrapDisabled = false; +} @Component({ @@ -106,4 +131,6 @@ class FocusTrapTestApp { } ` }) -class FocusTrapTargetTestApp { } +class FocusTrapTargetTestApp { + @ViewChild(FocusTrap) focusTrap: FocusTrap; +} diff --git a/src/lib/core/a11y/focus-trap.ts b/src/lib/core/a11y/focus-trap.ts index 714b7f6d3610..75e5e597b138 100644 --- a/src/lib/core/a11y/focus-trap.ts +++ b/src/lib/core/a11y/focus-trap.ts @@ -1,4 +1,4 @@ -import {Component, ViewEncapsulation, ViewChild, ElementRef, Input, NgZone} from '@angular/core'; +import {Directive, ElementRef, Input, NgZone, AfterViewInit, OnDestroy} from '@angular/core'; import {InteractivityChecker} from './interactivity-checker'; import {coerceBooleanProperty} from '../coercion/boolean-property'; @@ -11,31 +11,57 @@ import {coerceBooleanProperty} from '../coercion/boolean-property'; * Things like tabIndex > 0, flex `order`, and shadow roots can cause to two to misalign. * This will be replaced with a more intelligent solution before the library is considered stable. */ -@Component({ - moduleId: module.id, - selector: 'cdk-focus-trap, focus-trap', - templateUrl: 'focus-trap.html', - encapsulation: ViewEncapsulation.None, +@Directive({ + selector: 'cdk-focus-trap, focus-trap, [cdk-focus-trap], [focus-trap]', }) -export class FocusTrap { - @ViewChild('trappedContent') trappedContent: ElementRef; +export class FocusTrap implements AfterViewInit, OnDestroy { + private _startAnchor: HTMLElement = this._createAnchor(); + private _endAnchor: HTMLElement = this._createAnchor(); /** Whether the focus trap is active. */ @Input() get disabled(): boolean { return this._disabled; } - set disabled(val: boolean) { this._disabled = coerceBooleanProperty(val); } + set disabled(val: boolean) { + this._disabled = coerceBooleanProperty(val); + this._startAnchor.tabIndex = this._endAnchor.tabIndex = this._disabled ? -1 : 0; + } private _disabled: boolean = false; - constructor(private _checker: InteractivityChecker, private _ngZone: NgZone) { } + constructor( + private _checker: InteractivityChecker, + private _ngZone: NgZone, + private _elementRef: ElementRef) { } + + ngAfterViewInit() { + this._ngZone.runOutsideAngular(() => { + this._elementRef.nativeElement + .insertAdjacentElement('beforebegin', this._startAnchor) + .addEventListener('focus', () => this.focusLastTabbableElement()); + + this._elementRef.nativeElement + .insertAdjacentElement('afterend', this._endAnchor) + .addEventListener('focus', () => this.focusFirstTabbableElement()); + }); + } + + ngOnDestroy() { + if (this._startAnchor.parentNode) { + this._startAnchor.parentNode.removeChild(this._startAnchor); + } + + if (this._endAnchor.parentNode) { + this._endAnchor.parentNode.removeChild(this._endAnchor); + } + + this._startAnchor = this._endAnchor = null; + } /** * Waits for microtask queue to empty, then focuses the first tabbable element within the focus * trap region. */ focusFirstTabbableElementWhenReady() { - this._ngZone.onMicrotaskEmpty.first().subscribe(() => { - this.focusFirstTabbableElement(); - }); + this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusFirstTabbableElement()); } /** @@ -43,16 +69,14 @@ export class FocusTrap { * trap region. */ focusLastTabbableElementWhenReady() { - this._ngZone.onMicrotaskEmpty.first().subscribe(() => { - this.focusLastTabbableElement(); - }); + this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusLastTabbableElement()); } /** * Focuses the first tabbable element within the focus trap region. */ focusFirstTabbableElement() { - let rootElement = this.trappedContent.nativeElement; + let rootElement = this._elementRef.nativeElement; let redirectToElement = rootElement.querySelector('[cdk-focus-start]') as HTMLElement || this._getFirstTabbableElement(rootElement); @@ -65,14 +89,13 @@ export class FocusTrap { * Focuses the last tabbable element within the focus trap region. */ focusLastTabbableElement() { - let rootElement = this.trappedContent.nativeElement; - let focusTargets = rootElement.querySelectorAll('[cdk-focus-end]'); + let focusTargets = this._elementRef.nativeElement.querySelectorAll('[cdk-focus-end]'); let redirectToElement: HTMLElement = null; if (focusTargets.length) { redirectToElement = focusTargets[focusTargets.length - 1] as HTMLElement; } else { - redirectToElement = this._getLastTabbableElement(rootElement); + redirectToElement = this._getLastTabbableElement(this._elementRef.nativeElement); } if (redirectToElement) { @@ -114,4 +137,11 @@ export class FocusTrap { return null; } + + private _createAnchor(): HTMLElement { + let anchor = document.createElement('div'); + anchor.tabIndex = 0; + anchor.classList.add('cdk-visually-hidden'); + return anchor; + } } diff --git a/src/lib/sidenav/sidenav.scss b/src/lib/sidenav/sidenav.scss index 983df84838e0..201f2719848e 100644 --- a/src/lib/sidenav/sidenav.scss +++ b/src/lib/sidenav/sidenav.scss @@ -134,16 +134,13 @@ } .mat-sidenav-focus-trap { + box-sizing: border-box; height: 100%; + display: block; + overflow-y: auto; // TODO(kara): revisit scrolling behavior for sidenavs - > .cdk-focus-trap-content { - box-sizing: border-box; - height: 100%; - overflow-y: auto; // TODO(kara): revisit scrolling behavior for sidenavs - - // Prevents unnecessary repaints while scrolling. - transform: translateZ(0); - } + // Prevents unnecessary repaints while scrolling. + transform: translateZ(0); } .mat-sidenav-invalid {