From 905f7e1bc516b57c2a3b3589cffb840d473796e9 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 23 Feb 2017 19:36:37 +0100 Subject: [PATCH 1/2] refactor(focus-trap): convert to directive --- src/lib/core/a11y/focus-trap.html | 3 - src/lib/core/a11y/focus-trap.spec.ts | 67 +++++++--- src/lib/core/a11y/focus-trap.ts | 193 ++++++++++++++++++++++----- src/lib/core/a11y/index.ts | 8 +- src/lib/dialog/dialog-container.html | 4 +- src/lib/dialog/dialog-container.ts | 21 ++- src/lib/sidenav/sidenav.html | 4 +- src/lib/sidenav/sidenav.scss | 16 +-- src/lib/sidenav/sidenav.ts | 39 ++++-- 9 files changed, 258 insertions(+), 97 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..136137f8af84 100644 --- a/src/lib/core/a11y/focus-trap.spec.ts +++ b/src/lib/core/a11y/focus-trap.spec.ts @@ -1,7 +1,6 @@ -import {inject, ComponentFixture, TestBed, async} from '@angular/core/testing'; -import {By} from '@angular/platform-browser'; -import {Component} from '@angular/core'; -import {FocusTrap} from './focus-trap'; +import {ComponentFixture, TestBed, async} from '@angular/core/testing'; +import {Component, ViewChild} from '@angular/core'; +import {FocusTrapFactory, FocusTrapDirective, FocusTrap} from './focus-trap'; import {InteractivityChecker} from './interactivity-checker'; import {Platform} from '../platform/platform'; @@ -16,16 +15,15 @@ describe('FocusTrap', () => { beforeEach(async(() => { TestBed.configureTestingModule({ - declarations: [FocusTrap, FocusTrapTestApp], - providers: [InteractivityChecker, Platform] + declarations: [FocusTrapDirective, FocusTrapTestApp], + providers: [InteractivityChecker, Platform, FocusTrapFactory] }); TestBed.compileComponents(); - })); - beforeEach(inject([InteractivityChecker], (c: InteractivityChecker) => { fixture = TestBed.createComponent(FocusTrapTestApp); - focusTrapInstance = fixture.debugElement.query(By.directive(FocusTrap)).componentInstance; + fixture.detectChanges(); + focusTrapInstance = fixture.componentInstance.focusTrapDirective.focusTrap; })); it('wrap focus from end to start', () => { @@ -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.isFocusTrapEnabled = false; + fixture.detectChanges(); + + expect(anchors.every(current => current.getAttribute('tabindex') === '-1')).toBe(true); + }); }); describe('with focus targets', () => { @@ -56,16 +78,15 @@ describe('FocusTrap', () => { beforeEach(async(() => { TestBed.configureTestingModule({ - declarations: [FocusTrap, FocusTrapTargetTestApp], - providers: [InteractivityChecker, Platform] + declarations: [FocusTrapDirective, FocusTrapTargetTestApp], + providers: [InteractivityChecker, Platform, FocusTrapFactory] }); TestBed.compileComponents(); - })); - beforeEach(inject([InteractivityChecker], (c: InteractivityChecker) => { fixture = TestBed.createComponent(FocusTrapTargetTestApp); - focusTrapInstance = fixture.debugElement.query(By.directive(FocusTrap)).componentInstance; + fixture.detectChanges(); + focusTrapInstance = fixture.componentInstance.focusTrapDirective.focusTrap; })); it('should be able to prioritize the first focus target', () => { @@ -87,23 +108,29 @@ describe('FocusTrap', () => { @Component({ template: ` - +
- +
` }) -class FocusTrapTestApp { } +class FocusTrapTestApp { + @ViewChild(FocusTrapDirective) focusTrapDirective: FocusTrapDirective; + renderFocusTrap = true; + isFocusTrapEnabled = true; +} @Component({ template: ` - +
- +
` }) -class FocusTrapTargetTestApp { } +class FocusTrapTargetTestApp { + @ViewChild(FocusTrapDirective) focusTrapDirective: FocusTrapDirective; +} diff --git a/src/lib/core/a11y/focus-trap.ts b/src/lib/core/a11y/focus-trap.ts index 714b7f6d3610..f8289d42b75a 100644 --- a/src/lib/core/a11y/focus-trap.ts +++ b/src/lib/core/a11y/focus-trap.ts @@ -1,78 +1,122 @@ -import {Component, ViewEncapsulation, ViewChild, ElementRef, Input, NgZone} from '@angular/core'; +import { + Directive, + ElementRef, + Input, + NgZone, + OnDestroy, + AfterContentInit, + Injectable, +} from '@angular/core'; import {InteractivityChecker} from './interactivity-checker'; import {coerceBooleanProperty} from '../coercion/boolean-property'; /** - * Directive for trapping focus within a region. + * Class that allows for trapping focus within a DOM element. * - * NOTE: This directive currently uses a very simple (naive) approach to focus trapping. + * NOTE: This class currently uses a very simple (naive) approach to focus trapping. * It assumes that the tab order is the same as DOM order, which is not necessarily true. * 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, -}) export class FocusTrap { - @ViewChild('trappedContent') trappedContent: ElementRef; + private _startAnchor: HTMLElement; + private _endAnchor: HTMLElement; /** Whether the focus trap is active. */ - @Input() - get disabled(): boolean { return this._disabled; } - set disabled(val: boolean) { this._disabled = coerceBooleanProperty(val); } - private _disabled: boolean = false; + get enabled(): boolean { return this._enabled; } + set enabled(val: boolean) { + this._enabled = val; - constructor(private _checker: InteractivityChecker, private _ngZone: NgZone) { } + if (this._startAnchor && this._endAnchor) { + this._startAnchor.tabIndex = this._endAnchor.tabIndex = this._enabled ? 0 : -1; + } + } + private _enabled: boolean = true; + + constructor( + private _element: HTMLElement, + private _checker: InteractivityChecker, + private _ngZone: NgZone, + deferAnchors = false) { + + if (!deferAnchors) { + this.attachAnchors(); + } + } + + /** Destroys the focus trap by cleaning up the anchors. */ + destroy() { + if (this._startAnchor && this._startAnchor.parentNode) { + this._startAnchor.parentNode.removeChild(this._startAnchor); + } + + if (this._endAnchor && 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. + * Inserts the anchors into the DOM. This is usually done automatically + * in the constructor, but can be deferred for cases like directives with `*ngIf`. */ - focusFirstTabbableElementWhenReady() { - this._ngZone.onMicrotaskEmpty.first().subscribe(() => { - this.focusFirstTabbableElement(); + attachAnchors(): void { + if (!this._startAnchor) { + this._startAnchor = this._createAnchor(); + } + + if (!this._endAnchor) { + this._endAnchor = this._createAnchor(); + } + + this._ngZone.runOutsideAngular(() => { + this._element + .insertAdjacentElement('beforebegin', this._startAnchor) + .addEventListener('focus', () => this.focusLastTabbableElement()); + + this._element + .insertAdjacentElement('afterend', this._endAnchor) + .addEventListener('focus', () => this.focusFirstTabbableElement()); }); } /** - * Waits for microtask queue to empty, then focuses the last tabbable element within the focus - * trap region. + * Waits for microtask queue to empty, then focuses + * the first tabbable element within the focus trap region. */ - focusLastTabbableElementWhenReady() { - this._ngZone.onMicrotaskEmpty.first().subscribe(() => { - this.focusLastTabbableElement(); - }); + focusFirstTabbableElementWhenReady() { + this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusFirstTabbableElement()); } /** - * Focuses the first tabbable element within the focus trap region. + * Waits for microtask queue to empty, then focuses + * the last tabbable element within the focus trap region. */ + focusLastTabbableElementWhenReady() { + this._ngZone.onMicrotaskEmpty.first().subscribe(() => this.focusLastTabbableElement()); + } + + /** Focuses the first tabbable element within the focus trap region. */ focusFirstTabbableElement() { - let rootElement = this.trappedContent.nativeElement; - let redirectToElement = rootElement.querySelector('[cdk-focus-start]') as HTMLElement || - this._getFirstTabbableElement(rootElement); + let redirectToElement = this._element.querySelector('[cdk-focus-start]') as HTMLElement || + this._getFirstTabbableElement(this._element); if (redirectToElement) { redirectToElement.focus(); } } - /** - * Focuses the last tabbable element within the focus trap region. - */ + /** 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._element.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._element); } if (redirectToElement) { @@ -114,4 +158,81 @@ export class FocusTrap { return null; } + + /** Creates an anchor element. */ + private _createAnchor(): HTMLElement { + let anchor = document.createElement('div'); + anchor.tabIndex = this._enabled ? 0 : -1; + anchor.classList.add('cdk-visually-hidden'); + anchor.classList.add('cdk-focus-trap-anchor'); + return anchor; + } +} + + +/** Factory that allows easy instantiation of focus traps. */ +@Injectable() +export class FocusTrapFactory { + constructor(private _checker: InteractivityChecker, private _ngZone: NgZone) { } + + create(element: HTMLElement, deferAnchors = false): FocusTrap { + return new FocusTrap(element, this._checker, this._ngZone, deferAnchors); + } +} + + +/** + * Directive for trapping focus within a region. + * @deprecated + */ +@Directive({ + selector: 'cdk-focus-trap', +}) +export class FocusTrapDeprecatedDirective implements OnDestroy, AfterContentInit { + focusTrap: FocusTrap; + + /** Whether the focus trap is active. */ + @Input() + get disabled(): boolean { return !this.focusTrap.enabled; } + set disabled(val: boolean) { + this.focusTrap.enabled = !coerceBooleanProperty(val); + } + + constructor(private _elementRef: ElementRef, private _focusTrapFactory: FocusTrapFactory) { + this.focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement, true); + } + + ngOnDestroy() { + this.focusTrap.destroy(); + } + + ngAfterContentInit() { + this.focusTrap.attachAnchors(); + } +} + + +/** Directive for trapping focus within a region. */ +@Directive({ + selector: '[cdkTrapFocus]' +}) +export class FocusTrapDirective implements OnDestroy, AfterContentInit { + focusTrap: FocusTrap; + + /** Whether the focus trap is active. */ + @Input('cdkTrapFocus') + get enabled(): boolean { return this.focusTrap.enabled; } + set enabled(val: boolean) { this.focusTrap.enabled = val; } + + constructor(private _elementRef: ElementRef, private _focusTrapFactory: FocusTrapFactory) { + this.focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement, true); + } + + ngOnDestroy() { + this.focusTrap.destroy(); + } + + ngAfterContentInit() { + this.focusTrap.attachAnchors(); + } } diff --git a/src/lib/core/a11y/index.ts b/src/lib/core/a11y/index.ts index be37c92085c5..e222f173311b 100644 --- a/src/lib/core/a11y/index.ts +++ b/src/lib/core/a11y/index.ts @@ -1,5 +1,5 @@ import {NgModule, ModuleWithProviders} from '@angular/core'; -import {FocusTrap} from './focus-trap'; +import {FocusTrapDirective, FocusTrapDeprecatedDirective, FocusTrapFactory} from './focus-trap'; import {LIVE_ANNOUNCER_PROVIDER} from './live-announcer'; import {InteractivityChecker} from './interactivity-checker'; import {CommonModule} from '@angular/common'; @@ -7,9 +7,9 @@ import {PlatformModule} from '../platform/index'; @NgModule({ imports: [CommonModule, PlatformModule], - declarations: [FocusTrap], - exports: [FocusTrap], - providers: [InteractivityChecker, LIVE_ANNOUNCER_PROVIDER] + declarations: [FocusTrapDirective, FocusTrapDeprecatedDirective], + exports: [FocusTrapDirective, FocusTrapDeprecatedDirective], + providers: [InteractivityChecker, FocusTrapFactory, LIVE_ANNOUNCER_PROVIDER] }) export class A11yModule { /** @deprecated */ diff --git a/src/lib/dialog/dialog-container.html b/src/lib/dialog/dialog-container.html index cb0ea3ee0468..9ca2bad1e9cf 100644 --- a/src/lib/dialog/dialog-container.html +++ b/src/lib/dialog/dialog-container.html @@ -1,3 +1 @@ - - - + diff --git a/src/lib/dialog/dialog-container.ts b/src/lib/dialog/dialog-container.ts index bb54a4d79ac7..8816385f4272 100644 --- a/src/lib/dialog/dialog-container.ts +++ b/src/lib/dialog/dialog-container.ts @@ -5,6 +5,8 @@ import { ViewEncapsulation, NgZone, OnDestroy, + Renderer, + ElementRef, animate, state, style, @@ -16,7 +18,7 @@ import { import {BasePortalHost, ComponentPortal, PortalHostDirective, TemplatePortal} from '../core'; import {MdDialogConfig} from './dialog-config'; import {MdDialogContentAlreadyAttachedError} from './dialog-errors'; -import {FocusTrap} from '../core/a11y/focus-trap'; +import {FocusTrapFactory, FocusTrap} from '../core/a11y/focus-trap'; import 'rxjs/add/operator/first'; @@ -54,8 +56,8 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy { /** The portal host inside of this container into which the dialog content will be loaded. */ @ViewChild(PortalHostDirective) _portalHost: PortalHostDirective; - /** The directive that traps and manages focus within the dialog. */ - @ViewChild(FocusTrap) _focusTrap: FocusTrap; + /** The class that traps and manages focus within the dialog. */ + private _focusTrap: FocusTrap; /** Element that was focused before the dialog was opened. Save this to restore upon close. */ private _elementFocusedBeforeDialogWasOpened: HTMLElement = null; @@ -69,7 +71,12 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy { /** Emits the current animation state whenever it changes. */ _onAnimationStateChange = new EventEmitter(); - constructor(private _ngZone: NgZone) { + constructor( + private _ngZone: NgZone, + private _renderer: Renderer, + private _elementRef: ElementRef, + private _focusTrapFactory: FocusTrapFactory) { + super(); } @@ -106,6 +113,10 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy { * @private */ private _trapFocus() { + if (!this._focusTrap) { + this._focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement); + } + // If were to attempt to focus immediately, then the content of the dialog would not yet be // ready in instances where change detection has to run first. To deal with this, we simply // wait for the microtask queue to be empty. @@ -148,5 +159,7 @@ export class MdDialogContainer extends BasePortalHost implements OnDestroy { this._onAnimationStateChange.complete(); }); + + this._focusTrap.destroy(); } } diff --git a/src/lib/sidenav/sidenav.html b/src/lib/sidenav/sidenav.html index 43a820124c6e..6dbc74306383 100644 --- a/src/lib/sidenav/sidenav.html +++ b/src/lib/sidenav/sidenav.html @@ -1,3 +1 @@ - - - + diff --git a/src/lib/sidenav/sidenav.scss b/src/lib/sidenav/sidenav.scss index 983df84838e0..273299f5fcb0 100644 --- a/src/lib/sidenav/sidenav.scss +++ b/src/lib/sidenav/sidenav.scss @@ -100,6 +100,9 @@ z-index: 3; min-width: 5vw; outline: 0; + box-sizing: border-box; + height: 100%; + overflow-y: auto; // TODO(kara): revisit scrolling behavior for sidenavs @include mat-sidenav-transition(0, -100%); @@ -133,19 +136,6 @@ } } -.mat-sidenav-focus-trap { - height: 100%; - - > .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); - } -} - .mat-sidenav-invalid { display: none; } diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts index 4720e555a289..86f9fcf111d4 100644 --- a/src/lib/sidenav/sidenav.ts +++ b/src/lib/sidenav/sidenav.ts @@ -13,13 +13,13 @@ import { EventEmitter, Renderer, ViewEncapsulation, - ViewChild, - NgZone + NgZone, + OnDestroy, } from '@angular/core'; import {CommonModule} from '@angular/common'; import {Dir, MdError, coerceBooleanProperty, CompatibilityModule} from '../core'; import {A11yModule} from '../core/a11y/index'; -import {FocusTrap} from '../core/a11y/focus-trap'; +import {FocusTrapFactory, FocusTrap} from '../core/a11y/focus-trap'; import {ESCAPE} from '../core/keyboard/keycodes'; import {OverlayModule} from '../core/overlay/overlay-directives'; import 'rxjs/add/operator/first'; @@ -71,8 +71,8 @@ export class MdSidenavToggleResult { changeDetection: ChangeDetectionStrategy.OnPush, encapsulation: ViewEncapsulation.None, }) -export class MdSidenav implements AfterContentInit { - @ViewChild(FocusTrap) _focusTrap: FocusTrap; +export class MdSidenav implements AfterContentInit, OnDestroy { + private _focusTrap: FocusTrap; /** Alignment of the sidenav (direction neutral); whether 'start' or 'end'. */ private _align: 'start' | 'end' = 'start'; @@ -138,20 +138,24 @@ export class MdSidenav implements AfterContentInit { */ private _resolveToggleAnimationPromise: (animationFinished: boolean) => void = null; - get isFocusTrapDisabled() { + get isFocusTrapEnabled() { // The focus trap is only enabled when the sidenav is open in any mode other than side. - return !this.opened || this.mode == 'side'; + return this.opened && this.mode !== 'side'; } /** * @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 _renderer: Renderer) { + constructor( + private _elementRef: ElementRef, + private _renderer: Renderer, + private _focusTrapFactory: FocusTrapFactory) { + this.onOpen.subscribe(() => { this._elementFocusedBeforeSidenavWasOpened = document.activeElement as HTMLElement; - if (!this.isFocusTrapDisabled) { + if (this.isFocusTrapEnabled && this._focusTrap) { this._focusTrap.focusFirstTabbableElementWhenReady(); } }); @@ -168,14 +172,23 @@ export class MdSidenav implements AfterContentInit { } ngAfterContentInit() { - // This can happen when the sidenav is set to opened in the template and the transition - // isn't ended. + this._focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement); + this._focusTrap.enabled = this.isFocusTrapEnabled; + + // This can happen when the sidenav is set to opened in + // the template and the transition hasn't ended. if (this._toggleAnimationPromise) { this._resolveToggleAnimationPromise(true); this._toggleAnimationPromise = this._resolveToggleAnimationPromise = null; } } + ngOnDestroy() { + if (this._focusTrap) { + this._focusTrap.destroy(); + } + } + /** * Whether the sidenav is opened. We overload this because we trigger an event when it * starts or end. @@ -220,6 +233,10 @@ export class MdSidenav implements AfterContentInit { this._opened = isOpen; + if (this._focusTrap) { + this._focusTrap.enabled = this.isFocusTrapEnabled; + } + if (isOpen) { this.onOpenStart.emit(); } else { From 75d99a47044059e86b3d674bc0b179ac07710cb0 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Thu, 2 Mar 2017 07:58:09 +0100 Subject: [PATCH 2/2] fix: export new focus trap classes --- src/lib/core/core.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/core/core.ts b/src/lib/core/core.ts index 38aba4c9665f..b0bbbfcddd54 100644 --- a/src/lib/core/core.ts +++ b/src/lib/core/core.ts @@ -80,7 +80,7 @@ export * from './selection/selection'; /** @deprecated */ export {LiveAnnouncer as MdLiveAnnouncer} from './a11y/live-announcer'; -export {FocusTrap} from './a11y/focus-trap'; +export * from './a11y/focus-trap'; export {InteractivityChecker} from './a11y/interactivity-checker'; export {isFakeMousedownFromScreenReader} from './a11y/fake-mousedown';