fix(focus-trap): not attaching correctly if element is on in the DOM on init

Currently the focus trap attempts to attach itself once on init (e.g. when it is projected inside of an overlay), however in some cases it might not be in the DOM yet. These changes make it so it re-tries to attach itself until it does so successfully.

Author: crisbeto

src/cdk/a11y/focus-trap.ts

@@ -12,7 +12,7 @@ import {
   Input,
   NgZone,
   OnDestroy,
-  AfterContentInit,
+  DoCheck,
   Injectable,
 } from '@angular/core';
 import {coerceBooleanProperty} from '@angular/cdk/coercion';
@@ -32,6 +32,8 @@ import {InteractivityChecker} from './interactivity-checker';
 export class FocusTrap {
   private _startAnchor: HTMLElement | null;
   private _endAnchor: HTMLElement | null;
+  private _element: HTMLElement | null;
+  private _hasAttached = false;
 
   /** Whether the focus trap is active. */
   get enabled(): boolean { return this._enabled; }
@@ -45,12 +47,14 @@ export class FocusTrap {
   private _enabled: boolean = true;
 
   constructor(
-    private _element: HTMLElement,
+    element: HTMLElement,
     private _platform: Platform,
     private _checker: InteractivityChecker,
     private _ngZone: NgZone,
     deferAnchors = false) {
 
+    this._element = element;
+
     if (!deferAnchors) {
       this.attachAnchors();
     }
@@ -66,41 +70,41 @@ export class FocusTrap {
       this._endAnchor.parentNode.removeChild(this._endAnchor);
     }
 
-    this._startAnchor = this._endAnchor = null;
+    this._element = this._startAnchor = this._endAnchor = null;
   }
 
   /**
    * Inserts the anchors into the DOM. This is usually done automatically
    * in the constructor, but can be deferred for cases like directives with `*ngIf`.
+   * @returns Whether the focus trap managed to attach successfuly. This may not be the case
+   * if the target element isn't currently in the DOM.
    */
-  attachAnchors(): void {
+  attachAnchors(): boolean {
     // If we're not on the browser, there can be no focus to trap.
-    if (!this._platform.isBrowser) {
-      return;
-    }
-
-    if (!this._startAnchor) {
-      this._startAnchor = this._createAnchor();
-    }
-
-    if (!this._endAnchor) {
-      this._endAnchor = this._createAnchor();
+    if (!this._platform.isBrowser || this._hasAttached) {
+      this._hasAttached = true;
+      return true;
     }
 
     this._ngZone.runOutsideAngular(() => {
-      this._startAnchor!.addEventListener('focus', () => {
-        this.focusLastTabbableElement();
-      });
-
-      this._endAnchor!.addEventListener('focus', () => {
-        this.focusFirstTabbableElement();
-      });
+      if (!this._startAnchor) {
+        this._startAnchor = this._createAnchor();
+        this._startAnchor!.addEventListener('focus', () => this.focusLastTabbableElement());
+      }
 
-      if (this._element.parentNode) {
-        this._element.parentNode.insertBefore(this._startAnchor!, this._element);
-        this._element.parentNode.insertBefore(this._endAnchor!, this._element.nextSibling);
+      if (!this._endAnchor) {
+        this._endAnchor = this._createAnchor();
+        this._endAnchor!.addEventListener('focus', () => this.focusFirstTabbableElement());
       }
     });
+
+    if (this._element && this._element.parentNode) {
+      this._element.parentNode.insertBefore(this._startAnchor!, this._element);
+      this._element.parentNode.insertBefore(this._endAnchor!, this._element.nextSibling);
+      this._hasAttached = true;
+    }
+
+    return this._hasAttached;
   }
 
   /**
@@ -146,7 +150,7 @@ export class FocusTrap {
    */
   private _getRegionBoundary(bound: 'start' | 'end'): HTMLElement | null {
     // Contains the deprecated version of selector, for temporary backwards comparability.
-    let markers = this._element.querySelectorAll(`[cdk-focus-region-${bound}], ` +
+    let markers = this._element!.querySelectorAll(`[cdk-focus-region-${bound}], ` +
                                                  `[cdk-focus-${bound}]`) as NodeListOf<HTMLElement>;
 
     for (let i = 0; i < markers.length; i++) {
@@ -157,18 +161,18 @@ export class FocusTrap {
     }
 
     if (bound == 'start') {
-      return markers.length ? markers[0] : this._getFirstTabbableElement(this._element);
+      return markers.length ? markers[0] : this._getFirstTabbableElement(this._element!);
     }
     return markers.length ?
-        markers[markers.length - 1] : this._getLastTabbableElement(this._element);
+        markers[markers.length - 1] : this._getLastTabbableElement(this._element!);
   }
 
   /**
    * Focuses the element that should be focused when the focus trap is initialized.
    * @returns Returns whether focus was moved successfuly.
    */
   focusInitialElement(): boolean {
-    const redirectToElement = this._element.querySelector('[cdk-focus-initial]') as HTMLElement;
+    const redirectToElement = this._element!.querySelector('[cdk-focus-initial]') as HTMLElement;
 
     if (redirectToElement) {
       redirectToElement.focus();
@@ -206,6 +210,13 @@ export class FocusTrap {
     return !!redirectToElement;
   }
 
+  /**
+   * Checks whether the focus trap has successfuly been attached.
+   */
+  hasAttached(): boolean {
+    return this._hasAttached;
+  }
+
   /** Get the first tabbable element from a DOM subtree (inclusive). */
   private _getFirstTabbableElement(root: HTMLElement): HTMLElement | null {
     if (this._checker.isFocusable(root) && this._checker.isTabbable(root)) {
@@ -287,12 +298,12 @@ export class FocusTrapFactory {
 
 /**
  * Directive for trapping focus within a region.
- * @deprecated
+ * @deprecated Use `cdkTrapFocus` instead.
  */
 @Directive({
   selector: 'cdk-focus-trap',
 })
-export class FocusTrapDeprecatedDirective implements OnDestroy, AfterContentInit {
+export class FocusTrapDeprecatedDirective implements OnDestroy, DoCheck {
   focusTrap: FocusTrap;
 
   /** Whether the focus trap is active. */
@@ -310,8 +321,10 @@ export class FocusTrapDeprecatedDirective implements OnDestroy, AfterContentInit
     this.focusTrap.destroy();
   }
 
-  ngAfterContentInit() {
-    this.focusTrap.attachAnchors();
+  ngDoCheck() {
+    if (!this.focusTrap.hasAttached()) {
+      this.focusTrap.attachAnchors();
+    }
   }
 }
 
@@ -321,7 +334,7 @@ export class FocusTrapDeprecatedDirective implements OnDestroy, AfterContentInit
   selector: '[cdkTrapFocus]',
   exportAs: 'cdkTrapFocus',
 })
-export class FocusTrapDirective implements OnDestroy, AfterContentInit {
+export class FocusTrapDirective implements OnDestroy, DoCheck {
   focusTrap: FocusTrap;
 
   /** Whether the focus trap is active. */
@@ -337,7 +350,11 @@ export class FocusTrapDirective implements OnDestroy, AfterContentInit {
     this.focusTrap.destroy();
   }
 
-  ngAfterContentInit() {
-    this.focusTrap.attachAnchors();
+  ngDoCheck() {
+    // Since the element may not be attached to the DOM (or another element)
+    // immediately, keep trying until it is.
+    if (!this.focusTrap.hasAttached()) {
+      this.focusTrap.attachAnchors();
+    }
   }
 }