fix(cdk/portal): run change detection before inserting portal content

src/cdk/a11y/focus-trap/focus-trap.spec.ts

@@ -6,7 +6,7 @@ import {
   ViewContainerRef,
   ViewEncapsulation,
 } from '@angular/core';
-import {waitForAsync, ComponentFixture, TestBed} from '@angular/core/testing';
+import {waitForAsync, ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing';
 import {PortalModule, CdkPortalOutlet, TemplatePortal} from '@angular/cdk/portal';
 import {A11yModule, FocusTrap, CdkTrapFocus} from '../index';
 import {By} from '@angular/platform-browser';
@@ -91,10 +91,11 @@ describe('FocusTrap', () => {
   describe('with bindings', () => {
     let fixture: ComponentFixture<FocusTrapWithBindings>;
 
-    beforeEach(() => {
+    beforeEach(fakeAsync(() => {
       fixture = TestBed.createComponent(FocusTrapWithBindings);
       fixture.detectChanges();
-    });
+      tick();
+    }));
 
     it('should clean up its anchor sibling elements on destroy', () => {
       const rootElement = fixture.debugElement.nativeElement as HTMLElement;
@@ -298,26 +299,51 @@ describe('FocusTrap', () => {
 
   });
 
-  it('should put anchors inside the outlet when set at the root of a template portal', () => {
-    const fixture = TestBed.createComponent(FocusTrapInsidePortal);
-    const instance = fixture.componentInstance;
-    fixture.detectChanges();
-    const outlet: HTMLElement = fixture.nativeElement.querySelector('.portal-outlet');
-
-    expect(outlet.querySelectorAll('button').length)
-      .withContext('Expected no buttons inside the outlet on init.').toBe(0);
-    expect(outlet.querySelectorAll('.cdk-focus-trap-anchor').length)
-      .withContext('Expected no focus trap anchors inside the outlet on init.').toBe(0);
-
-    const portal = new TemplatePortal(instance.template, instance.viewContainerRef);
-    instance.portalOutlet.attachTemplatePortal(portal);
-    fixture.detectChanges();
-
-    expect(outlet.querySelectorAll('button').length)
-      .withContext('Expected one button inside the outlet after attaching.').toBe(1);
-    expect(outlet.querySelectorAll('.cdk-focus-trap-anchor').length)
-      .withContext('Expected two focus trap anchors in the outlet after attaching.').toBe(2);
-  });
+  it('should put anchors inside the outlet when set at the root of a template portal',
+    fakeAsync(() => {
+      const fixture = TestBed.createComponent(FocusTrapInsidePortal);
+      const instance = fixture.componentInstance;
+      fixture.detectChanges();
+      const outlet: HTMLElement = fixture.nativeElement.querySelector('.portal-outlet');
+
+      expect(outlet.querySelectorAll('button').length)
+        .withContext('Expected no buttons inside the outlet on init.').toBe(0);
+      expect(outlet.querySelectorAll('.cdk-focus-trap-anchor').length)
+        .withContext('Expected no focus trap anchors inside the outlet on init.').toBe(0);
+
+      const portal = new TemplatePortal(instance.template, instance.viewContainerRef);
+      instance.portalOutlet.attachTemplatePortal(portal);
+      fixture.detectChanges();
+      tick();
+
+      expect(outlet.querySelectorAll('button').length)
+        .withContext('Expected one button inside the outlet after attaching.').toBe(1);
+      expect(outlet.querySelectorAll('.cdk-focus-trap-anchor').length)
+        .withContext('Expected two focus trap anchors in the outlet after attaching.').toBe(2);
+    }));
+
+  it('should put anchors inside the outlet when set at the root of a template portal',
+    fakeAsync(() => {
+      const fixture = TestBed.createComponent(FocusTrapInsidePortal);
+      const instance = fixture.componentInstance;
+      fixture.detectChanges();
+      const outlet: HTMLElement = fixture.nativeElement.querySelector('.portal-outlet');
+
+      expect(outlet.querySelectorAll('button').length)
+        .withContext('Expected no buttons inside the outlet on init.').toBe(0);
+      expect(outlet.querySelectorAll('.cdk-focus-trap-anchor').length)
+        .withContext('Expected no focus trap anchors inside the outlet on init.').toBe(0);
+
+      const portal = new TemplatePortal(instance.template, instance.viewContainerRef);
+      instance.portalOutlet.attachTemplatePortal(portal);
+      fixture.detectChanges();
+      tick();
+
+      expect(outlet.querySelectorAll('button').length)
+        .withContext('Expected one button inside the outlet after attaching.').toBe(1);
+      expect(outlet.querySelectorAll('.cdk-focus-trap-anchor').length)
+        .withContext('Expected two focus trap anchors in the outlet after attaching.').toBe(2);
+    }));
 });
 
 /** Gets the currently-focused element while accounting for the shadow DOM. */

src/cdk/a11y/focus-trap/focus-trap.ts

@@ -392,6 +392,9 @@ export class CdkTrapFocus implements OnDestroy, AfterContentInit, OnChanges, DoC
   /** Previously focused element to restore focus to upon destroy when using autoCapture. */
   private _previouslyFocusedElement: HTMLElement | null = null;
 
+  /** Whether the focus trap has been initialized. */
+  private _hasInitialized: boolean;
+
   /** Whether the focus trap is active. */
   @Input('cdkTrapFocus')
   get enabled(): boolean { return this.focusTrap.enabled; }
@@ -413,7 +416,12 @@ export class CdkTrapFocus implements OnDestroy, AfterContentInit, OnChanges, DoC
        * @deprecated No longer being used. To be removed.
        * @breaking-change 13.0.0
        */
-      @Inject(DOCUMENT) _document: any) {
+      @Inject(DOCUMENT) _document: any,
+      /**
+       * @deprecated _ngZone parameter to become required.
+       * @breaking-change 14.0.0
+       */
+       private _ngZone?: NgZone) {
     this.focusTrap = this._focusTrapFactory.create(this._elementRef.nativeElement, true);
   }
 
@@ -429,15 +437,32 @@ export class CdkTrapFocus implements OnDestroy, AfterContentInit, OnChanges, DoC
   }
 
   ngAfterContentInit() {
-    this.focusTrap.attachAnchors();
-
-    if (this.autoCapture) {
-      this._captureFocus();
+    // We have to delay attaching the anchors, because the lifecycle hook may be fired before the
+    // focus trap element is at its final place in the DOM when inserted via a portal (see #16346).
+    // It has to happen outside the `NgZone`, because the promise can delay `NgZone.onStable` which
+    // overlays depend on for positioning.
+    const attachAnchor = () => {
+      Promise.resolve().then(() => {
+        this.focusTrap.attachAnchors();
+
+        if (this.autoCapture) {
+          this._captureFocus();
+        }
+
+        this._hasInitialized = true;
+      });
+    };
+
+    // @breaking-change 11.0.0 Remove null check for `_ngZone`.
+    if (this._ngZone) {
+      this._ngZone.runOutsideAngular(attachAnchor);
+    } else {
+      attachAnchor();
     }
   }
 
   ngDoCheck() {
-    if (!this.focusTrap.hasAttached()) {
+    if (this._hasInitialized && !this.focusTrap.hasAttached()) {
       this.focusTrap.attachAnchors();
     }
   }

src/cdk/overlay/overlay.spec.ts

@@ -15,6 +15,8 @@ import {
   Injectable,
   EventEmitter,
   NgZone,
+  ComponentFactoryResolver,
+  TemplateRef,
 } from '@angular/core';
 import {Direction, Directionality} from '@angular/cdk/bidi';
 import {MockNgZone, dispatchFakeEvent} from '../testing/private';
@@ -1032,6 +1034,31 @@ describe('Overlay', () => {
     }));
 
   });
+
+  it('should insert component that adds content through ViewContainerRef', () => {
+    componentPortal = new ComponentPortal(FoodRenderer);
+    const overlayRef = overlay.create();
+    overlayRef.attach(componentPortal);
+
+    expect(overlayContainerElement.textContent).toContain('Food:Pizza');
+    overlayRef.dispose();
+  });
+
+  it('should insert template that has component that adds content via ViewContainerRef', () => {
+    const fixture = TestBed.createComponent(FoodRendererInTemplate);
+    fixture.detectChanges();
+
+    const portal = new TemplatePortal(
+      fixture.componentInstance.foodRendererTemplate,
+      fixture.componentInstance.viewContainerRef
+    );
+    const overlayRef = overlay.create();
+    overlayRef.attach(portal);
+
+    expect(overlayContainerElement.textContent).toContain('Food:Pizza');
+    overlayRef.dispose();
+  });
+
 });
 
 /** Simple component for testing ComponentPortal. */
@@ -1050,9 +1077,42 @@ class TestComponentWithTemplatePortals {
   constructor(public viewContainerRef: ViewContainerRef) { }
 }
 
+@Component({
+  selector: 'food-renderer',
+  template: '<p>Food:</p>',
+})
+class FoodRenderer {
+  constructor(
+    private _componentFactoryResolver: ComponentFactoryResolver,
+    private _viewContainerRef: ViewContainerRef) {
+      const factory = this._componentFactoryResolver.resolveComponentFactory(PizzaMsg);
+      this._viewContainerRef.clear();
+      this._viewContainerRef.createComponent(factory);
+  }
+}
+
+@Component({
+  template: `
+    <ng-template #foodRenderer>
+      <food-renderer></food-renderer>
+    </ng-template>
+  `
+})
+class FoodRendererInTemplate {
+  @ViewChild('foodRenderer') foodRendererTemplate: TemplateRef<any>;
+
+  constructor(public viewContainerRef: ViewContainerRef) { }
+}
+
 // Create a real (non-test) NgModule as a workaround for
 // https://github.com/angular/angular/issues/10760
-const TEST_COMPONENTS = [PizzaMsg, TestComponentWithTemplatePortals];
+const TEST_COMPONENTS = [
+  PizzaMsg,
+  TestComponentWithTemplatePortals,
+  FoodRenderer,
+  FoodRendererInTemplate,
+];
+
 @NgModule({
   imports: [OverlayModule, PortalModule],
   exports: TEST_COMPONENTS,

src/cdk/portal/dom-portal-outlet.ts

@@ -70,7 +70,7 @@ export class DomPortalOutlet extends BasePortalOutlet {
     }
     // At this point the component has been instantiated, so we move it to the location in the DOM
     // where we want it to be rendered.
-    this.outletElement.appendChild(this._getComponentRootNode(componentRef));
+    this._transferRootNodes(componentRef.hostView as EmbeddedViewRef<any>);
     this._attachedPortal = portal;
 
     return componentRef;
@@ -82,23 +82,23 @@ export class DomPortalOutlet extends BasePortalOutlet {
    * @returns Reference to the created embedded view.
    */
   attachTemplatePortal<C>(portal: TemplatePortal<C>): EmbeddedViewRef<C> {
-    let viewContainer = portal.viewContainerRef;
-    let viewRef = viewContainer.createEmbeddedView(portal.templateRef, portal.context);
+    const viewContainer = portal.viewContainerRef;
+    const viewRef = viewContainer.createEmbeddedView(portal.templateRef, portal.context);
+
+    // Trigger change detection so that we invoke the lifecycle hooks of any components inside the
+    // template **before** moving the DOM nodes into their new position, because it can have an
+    // effect on the amount of `rootNodes` (e.g. if something is inserted via a `ViewContainerRef`).
+    viewRef.detectChanges();
 
     // The method `createEmbeddedView` will add the view as a child of the viewContainer.
     // But for the DomPortalOutlet the view can be added everywhere in the DOM
     // (e.g Overlay Container) To move the view to the specified host element. We just
     // re-append the existing root nodes.
-    viewRef.rootNodes.forEach(rootNode => this.outletElement.appendChild(rootNode));
-
-    // Note that we want to detect changes after the nodes have been moved so that
-    // any directives inside the portal that are looking at the DOM inside a lifecycle
-    // hook won't be invoked too early.
-    viewRef.detectChanges();
+    this._transferRootNodes(viewRef);
 
     this.setDisposeFn((() => {
-      let index = viewContainer.indexOf(viewRef);
-      if (index !== -1) {
+      const index = viewContainer.indexOf(viewRef);
+      if (index > -1) {
         viewContainer.remove(index);
       }
     }));
@@ -153,9 +153,9 @@ export class DomPortalOutlet extends BasePortalOutlet {
     }
   }
 
-  /** Gets the root HTMLElement for an instantiated component. */
-  private _getComponentRootNode(componentRef: ComponentRef<any>): HTMLElement {
-    return (componentRef.hostView as EmbeddedViewRef<any>).rootNodes[0] as HTMLElement;
+  /** Moves all of the root nodes of an embedded view into the outlet element. */
+  private _transferRootNodes(viewRef: EmbeddedViewRef<any>): void {
+    viewRef.rootNodes.forEach(node => this.outletElement.appendChild(node));
   }
 }
 

src/cdk/portal/portal.spec.ts

@@ -14,8 +14,6 @@ import {
   ViewChild,
   ViewChildren,
   ViewContainerRef,
-  Directive,
-  AfterViewInit,
 } from '@angular/core';
 import {ComponentFixture, inject, TestBed} from '@angular/core/testing';
 import {DomPortalOutlet} from './dom-portal-outlet';
@@ -448,19 +446,6 @@ describe('Portals', () => {
       expect(someDomElement.innerHTML).toBe('');
     });
 
-    it('should move the DOM nodes before running change detection', () => {
-      someFixture.detectChanges();
-      let portal = new TemplatePortal(someFixture.componentInstance.template, someViewContainerRef);
-
-      host.attachTemplatePortal(portal);
-      someFixture.detectChanges();
-
-      expect(someFixture.componentInstance.saveParentNodeOnInit.parentOnViewInit)
-          .toBe(someDomElement);
-
-      host.dispose();
-    });
-
     it('should attach and detach a component portal with a given injector', () => {
       let fixture = TestBed.createComponent(ArbitraryViewContainerRefComponent);
       someViewContainerRef = fixture.componentInstance.viewContainerRef;
@@ -682,37 +667,15 @@ class PizzaMsg {
   constructor(@Optional() public snack: Chocolate) { }
 }
 
-/**
- * Saves the parent node that the directive was attached to on init.
- * Useful to see where the element was in the DOM when it was first attached.
- */
-@Directive({
-  selector: '[savesParentNodeOnInit]'
-})
-class SaveParentNodeOnInit implements AfterViewInit {
-  parentOnViewInit: HTMLElement;
-
-  constructor(private _elementRef: ElementRef<HTMLElement>) {}
-
-  ngAfterViewInit() {
-    this.parentOnViewInit = this._elementRef.nativeElement.parentElement!;
-  }
-}
-
 /** Simple component to grab an arbitrary ViewContainerRef */
 @Component({
   selector: 'some-placeholder',
   template: `
     <p>Hello</p>
-
-    <ng-template #template>
-      <div savesParentNodeOnInit></div>
-    </ng-template>
   `
 })
 class ArbitraryViewContainerRefComponent {
   @ViewChild('template') template: TemplateRef<any>;
-  @ViewChild(SaveParentNodeOnInit) saveParentNodeOnInit: SaveParentNodeOnInit;
 
   constructor(public viewContainerRef: ViewContainerRef, public injector: Injector) { }
 }
@@ -805,7 +768,7 @@ const TEST_COMPONENTS = [
 @NgModule({
   imports: [CommonModule, PortalModule],
   exports: TEST_COMPONENTS,
-  declarations: [...TEST_COMPONENTS, SaveParentNodeOnInit],
+  declarations: TEST_COMPONENTS,
   entryComponents: TEST_COMPONENTS,
 })
 class PortalTestModule { }

tools/public_api_guard/cdk/a11y.md

@@ -95,7 +95,8 @@ export class CdkMonitorFocus implements AfterViewInit, OnDestroy {
 // @public
 export class CdkTrapFocus implements OnDestroy, AfterContentInit, OnChanges, DoCheck {
     constructor(_elementRef: ElementRef<HTMLElement>, _focusTrapFactory: FocusTrapFactory,
-    _document: any);
+    _document: any,
+    _ngZone?: NgZone | undefined);
     get autoCapture(): boolean;
     set autoCapture(value: boolean);
     get enabled(): boolean;