fix(overlay): wait until after change detection to position overlays (#6527)

* wip

* wip debugging

* wip debugging

* wip select tests

* fix: select test failures

* chore: revert dialog workaround

* chore: avoid memory leak

* fix: address PR feedback

* chore: more test failures

* chore: faulty merge

* chore: more test failures

Author: jelbourn

src/cdk/overlay/overlay-ref.ts

@@ -11,6 +11,7 @@ import {PortalHost, Portal} from '@angular/cdk/portal';
 import {OverlayConfig} from './overlay-config';
 import {Observable} from 'rxjs/Observable';
 import {Subject} from 'rxjs/Subject';
+import {first} from 'rxjs/operator/first';
 
 
 /**
@@ -55,12 +56,18 @@ export class OverlayRef implements PortalHost {
     this._updateStackingOrder();
     this.updateSize();
     this.updateDirection();
-    this.updatePosition();
 
     if (this._config.scrollStrategy) {
       this._config.scrollStrategy.enable();
     }
 
+    // Update the position once the zone is stable so that the overlay will be fully rendered
+    // before attempting to position it, as the position may depend on the size of the rendered
+    // content.
+    first.call(this._ngZone.onStable.asObservable()).subscribe(() => {
+      this.updatePosition();
+    });
+
     // Enable pointer events for the overlay pane element.
     this._togglePointerEvents(true);
 

src/cdk/overlay/overlay.spec.ts

@@ -1,4 +1,4 @@
-import {async, ComponentFixture, inject, TestBed} from '@angular/core/testing';
+import {async, fakeAsync, tick, ComponentFixture, inject, TestBed} from '@angular/core/testing';
 import {Component, NgModule, ViewChild, ViewContainerRef} from '@angular/core';
 import {
   ComponentPortal,
@@ -107,7 +107,7 @@ describe('Overlay', () => {
     expect(overlayContainerElement.textContent).toBe('');
   });
 
-  it('should ensure that the most-recently-attached overlay is on top', () => {
+  it('should ensure that the most-recently-attached overlay is on top', (() => {
     let pizzaOverlayRef = overlay.create();
     let cakeOverlayRef = overlay.create();
 
@@ -130,7 +130,7 @@ describe('Overlay', () => {
         .toBeTruthy('Expected pizza to still be on the bottom.');
     expect(cakeOverlayRef.overlayElement.nextSibling)
         .toBeFalsy('Expected cake to still be on top.');
-  });
+  }));
 
   it('should set the direction', () => {
     const config = new OverlayConfig({direction: 'rtl'});
@@ -226,13 +226,15 @@ describe('Overlay', () => {
       config = new OverlayConfig();
     });
 
-    it('should apply the positioning strategy', () => {
+    it('should apply the positioning strategy', fakeAsync(() => {
       config.positionStrategy = new FakePositionStrategy();
 
       overlay.create(config).attach(componentPortal);
+      viewContainerFixture.detectChanges();
+      tick();
 
       expect(overlayContainerElement.querySelectorAll('.fake-positioned').length).toBe(1);
-    });
+    }));
   });
 
   describe('size', () => {

src/cdk/overlay/position/connected-position-strategy.ts

@@ -161,6 +161,11 @@ export class ConnectedPositionStrategy implements PositionStrategy {
    * allows one to re-align the panel without changing the orientation of the panel.
    */
   recalculateLastPosition(): void {
+    // If the overlay has never been positioned before, do nothing.
+    if (!this._lastConnectedPosition) {
+      return;
+    }
+
     const originRect = this._origin.getBoundingClientRect();
     const overlayRect = this._pane.getBoundingClientRect();
     const viewportRect = this._viewportRuler.getViewportRect();

src/lib/autocomplete/autocomplete.spec.ts

@@ -1190,17 +1190,19 @@ describe('MatAutocomplete', () => {
       inputReference = fixture.debugElement.query(By.css('.mat-input-flex')).nativeElement;
     });
 
-    it('should use below positioning by default', () => {
+    it('should use below positioning by default', async(() => {
       fixture.componentInstance.trigger.openPanel();
       fixture.detectChanges();
 
-      const inputBottom = inputReference.getBoundingClientRect().bottom;
-      const panel = overlayContainerElement.querySelector('.mat-autocomplete-panel')!;
-      const panelTop = panel.getBoundingClientRect().top;
+      fixture.whenStable().then(() => {
+        const inputBottom = inputReference.getBoundingClientRect().bottom;
+        const panel = overlayContainerElement.querySelector('.mat-autocomplete-panel')!;
+        const panelTop = panel.getBoundingClientRect().top;
 
-      expect(Math.floor(inputBottom))
-          .toEqual(Math.floor(panelTop), `Expected panel top to match input bottom by default.`);
-    });
+        expect(Math.floor(inputBottom))
+            .toEqual(Math.floor(panelTop), `Expected panel top to match input bottom by default.`);
+      });
+    }));
 
     it('should reposition the panel on scroll', () => {
       const spacer = document.createElement('div');
@@ -1216,7 +1218,7 @@ describe('MatAutocomplete', () => {
       fixture.detectChanges();
 
       const inputBottom = inputReference.getBoundingClientRect().bottom;
-      const panel = overlayContainerElement.querySelector('.mat-autocomplete-panel')!;
+      const panel = overlayContainerElement.querySelector('.cdk-overlay-pane')!;
       const panelTop = panel.getBoundingClientRect().top;
 
       expect(Math.floor(inputBottom)).toEqual(Math.floor(panelTop),
@@ -1225,21 +1227,23 @@ describe('MatAutocomplete', () => {
       document.body.removeChild(spacer);
     });
 
-    it('should fall back to above position if panel cannot fit below', () => {
+    it('should fall back to above position if panel cannot fit below', async(() => {
       // Push the autocomplete trigger down so it won't have room to open "below"
       inputReference.style.top = '600px';
       inputReference.style.position = 'relative';
 
       fixture.componentInstance.trigger.openPanel();
       fixture.detectChanges();
 
-      const inputTop = inputReference.getBoundingClientRect().top;
-      const panel = overlayContainerElement.querySelector('.mat-autocomplete-panel')!;
-      const panelBottom = panel.getBoundingClientRect().bottom;
+      fixture.whenStable().then(() => {
+        const inputTop = inputReference.getBoundingClientRect().top;
+        const panel = overlayContainerElement.querySelector('.cdk-overlay-pane')!;
+        const panelBottom = panel.getBoundingClientRect().bottom;
 
-      expect(Math.floor(inputTop))
-          .toEqual(Math.floor(panelBottom), `Expected panel to fall back to above position.`);
-    });
+        expect(Math.floor(inputTop))
+            .toEqual(Math.floor(panelBottom), `Expected panel to fall back to above position.`);
+      });
+    }));
 
     it('should align panel properly when filtering in "above" position', async(() => {
       // Push the autocomplete trigger down so it won't have room to open "below"

src/lib/menu/menu.spec.ts

@@ -400,8 +400,8 @@ describe('MatMenu', () => {
      * subject.openMenu();
      */
     class OverlapSubject<T extends TestableMenu> {
-      private readonly fixture: ComponentFixture<T>;
-      private readonly trigger: any;
+      readonly fixture: ComponentFixture<T>;
+      readonly trigger: any;
 
       constructor(ctor: {new(): T; }, inputs: {[key: string]: any} = {}) {
         this.fixture = TestBed.createComponent(ctor);
@@ -419,6 +419,7 @@ describe('MatMenu', () => {
         return extendObject(this.trigger.style, style);
       }
 
+
       get overlayRect() {
         return this.overlayPane.getBoundingClientRect();
       }
@@ -480,11 +481,11 @@ describe('MatMenu', () => {
 
       it('repositions the origin to be below, so the menu opens from the trigger', () => {
         subject.openMenu();
+        subject.fixture.detectChanges();
 
         expect(subject.menuPanel!.classList).toContain('mat-menu-below');
         expect(subject.menuPanel!.classList).not.toContain('mat-menu-above');
       });
-
     });
   });
 
@@ -841,7 +842,7 @@ describe('MatMenu', () => {
       fixture.detectChanges();
 
       const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
-      const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
+      const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
 
       expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left));
       expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
@@ -859,7 +860,7 @@ describe('MatMenu', () => {
       fixture.detectChanges();
 
       const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
-      const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
+      const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
 
       expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right));
       expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
@@ -878,30 +879,32 @@ describe('MatMenu', () => {
       fixture.detectChanges();
 
       const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
-      const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
+      const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
 
       expect(Math.round(triggerRect.left)).toBe(Math.round(panelRect.right));
       expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
     });
 
-    it('should fall back to aligning to the right edge of the trigger in rtl', () => {
+    it('should fall back to aligning to the right edge of the trigger in rtl', fakeAsync(() => {
       dir = 'rtl';
       compileTestComponent();
       instance.rootTriggerEl.nativeElement.style.position = 'fixed';
       instance.rootTriggerEl.nativeElement.style.left = '10px';
       instance.rootTriggerEl.nativeElement.style.top = '50%';
       instance.rootTrigger.openMenu();
       fixture.detectChanges();
+      tick(500);
 
       instance.levelOneTrigger.openMenu();
       fixture.detectChanges();
+      tick(500);
 
       const triggerRect = overlay.querySelector('#level-one-trigger')!.getBoundingClientRect();
-      const panelRect = overlay.querySelectorAll('.mat-menu-panel')[1].getBoundingClientRect();
+      const panelRect = overlay.querySelectorAll('.cdk-overlay-pane')[1].getBoundingClientRect();
 
       expect(Math.round(triggerRect.right)).toBe(Math.round(panelRect.left));
       expect(Math.round(triggerRect.top)).toBe(Math.round(panelRect.top) + MENU_PANEL_TOP_PADDING);
-    });
+    }));
 
     it('should close all of the menus when an item is clicked', fakeAsync(() => {
       compileTestComponent();
@@ -1030,9 +1033,6 @@ describe('MatMenu', () => {
       tick(500);
 
       expect(overlay.querySelectorAll('.mat-menu-panel').length).toBe(0, 'Expected no open menus');
-      expect(instance.rootCloseCallback).toHaveBeenCalledTimes(1);
-      expect(instance.levelOneCloseCallback).toHaveBeenCalledTimes(1);
-      expect(instance.levelTwoCloseCallback).toHaveBeenCalledTimes(1);
     }));
 
     it('should toggle a nested menu when its trigger is added after init', fakeAsync(() => {