perf(overlay): remove detached overlays from the DOM

Removes the detached overlays from the DOM, rather than keeping their host element. Previously we kept the elements in the DOM, because they don't interfere with user interactions and it made it easier to handle animations, however doing so can cause scroll jank, because the host still has dimensions and is being rendered by the browser.

Fixes #12341.

Author: crisbeto

src/cdk/overlay/overlay-directives.spec.ts

@@ -51,15 +51,11 @@ describe('Overlay directives', () => {
     fixture.detectChanges();
 
     expect(overlayContainerElement.textContent).toContain('Menu content');
-    expect(getPaneElement().style.pointerEvents)
-      .toBe('auto', 'Expected the overlay pane to enable pointerEvents when attached.');
 
     fixture.componentInstance.isOpen = false;
     fixture.detectChanges();
 
     expect(overlayContainerElement.textContent).toBe('');
-    expect(getPaneElement().style.pointerEvents)
-      .toBe('none', 'Expected the overlay pane to disable pointerEvents when detached.');
   });
 
   it('should destroy the overlay when the directive is destroyed', () => {

src/cdk/overlay/overlay-ref.ts

@@ -9,8 +9,8 @@
 import {Direction, Directionality} from '@angular/cdk/bidi';
 import {ComponentPortal, Portal, PortalOutlet, TemplatePortal} from '@angular/cdk/portal';
 import {ComponentRef, EmbeddedViewRef, NgZone} from '@angular/core';
-import {Observable, Subject} from 'rxjs';
-import {take} from 'rxjs/operators';
+import {Observable, Subject, merge} from 'rxjs';
+import {take, takeUntil} from 'rxjs/operators';
 import {OverlayKeyboardDispatcher} from './keyboard/overlay-keyboard-dispatcher';
 import {OverlayConfig} from './overlay-config';
 import {coerceCssPixelValue, coerceArray} from '@angular/cdk/coercion';
@@ -31,6 +31,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
   private _backdropClick: Subject<MouseEvent> = new Subject();
   private _attachments = new Subject<void>();
   private _detachments = new Subject<void>();
+  private _previousHostParent: HTMLElement;
   private _keydownEventsObservable: Observable<KeyboardEvent> = Observable.create(observer => {
     const subscription = this._keydownEvents.subscribe(observer);
     this._keydownEventSubscriptions++;
@@ -99,6 +100,10 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
     }
 
     // Update the pane element with the given configuration.
+    if (!this._host.parentElement && this._previousHostParent) {
+      this._previousHostParent.appendChild(this._host);
+    }
+
     this._updateStackingOrder();
     this._updateElementSize();
     this._updateElementDirection();
@@ -176,6 +181,26 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
     // Remove this overlay from keyboard dispatcher tracking.
     this._keyboardDispatcher.remove(this);
 
+    // Keeping the host element in DOM the can cause scroll jank, because it still gets rendered,
+    // even though it's transparent and unclickable. We can't remove the host here immediately,
+    // because the overlay pane's content might still be animating. This stream helps us avoid
+    // interrupting the animation by waiting for the pane to become empty.
+    const subscription = this._ngZone.onStable
+      .asObservable()
+      .pipe(takeUntil(merge(this._attachments, this._detachments)))
+      .subscribe(() => {
+        // Needs a couple of checks for the pane and host, because
+        // they may have been removed by the time the zone stabilizes.
+        if (!this._pane || !this._host || this._pane.children.length === 0) {
+          if (this._host && this._host.parentElement) {
+            this._previousHostParent = this._host.parentElement;
+            this._previousHostParent.removeChild(this._host);
+          }
+
+          subscription.unsubscribe();
+        }
+      });
+
     return detachmentResult;
   }
 
@@ -203,7 +228,7 @@ export class OverlayRef implements PortalOutlet, OverlayReference {
       this._host = null!;
     }
 
-    this._pane = null!;
+    this._previousHostParent = this._pane = null!;
 
     if (isAttached) {
       this._detachments.next();

src/cdk/overlay/overlay.spec.ts

@@ -7,9 +7,10 @@ import {
   ErrorHandler,
   Injectable,
   EventEmitter,
+  NgZone,
 } from '@angular/core';
 import {Direction, Directionality} from '@angular/cdk/bidi';
-import {dispatchFakeEvent} from '@angular/cdk/testing';
+import {dispatchFakeEvent, MockNgZone} from '@angular/cdk/testing';
 import {
   ComponentPortal,
   PortalModule,
@@ -35,19 +36,26 @@ describe('Overlay', () => {
   let overlayContainer: OverlayContainer;
   let viewContainerFixture: ComponentFixture<TestComponentWithTemplatePortals>;
   let dir: Direction;
+  let zone: MockNgZone;
 
   beforeEach(async(() => {
     dir = 'ltr';
     TestBed.configureTestingModule({
       imports: [OverlayModule, PortalModule, OverlayTestModule],
-      providers: [{
-        provide: Directionality,
-        useFactory: () => {
-          const fakeDirectionality = {};
-          Object.defineProperty(fakeDirectionality, 'value', {get: () => dir});
-          return fakeDirectionality;
-        }
-      }],
+      providers: [
+        {
+          provide: Directionality,
+          useFactory: () => {
+            const fakeDirectionality = {};
+            Object.defineProperty(fakeDirectionality, 'value', {get: () => dir});
+            return fakeDirectionality;
+          }
+        },
+        {
+          provide: NgZone,
+          useFactory: () => zone = new MockNgZone()
+        },
+      ],
     }).compileComponents();
   }));
 
@@ -342,6 +350,33 @@ describe('Overlay', () => {
     expect(overlayRef.getDirection()).toBe('ltr');
   });
 
+  it('should add and remove the overlay host as the ref is being attached and detached', () => {
+    const overlayRef = overlay.create();
+
+    overlayRef.attach(componentPortal);
+    viewContainerFixture.detectChanges();
+
+    expect(overlayRef.hostElement.parentElement)
+        .toBeTruthy('Expected host element to be in the DOM.');
+
+    overlayRef.detach();
+
+    expect(overlayRef.hostElement.parentElement)
+        .toBeTruthy('Expected host element not to have been removed immediately.');
+
+    viewContainerFixture.detectChanges();
+    zone.simulateZoneExit();
+
+    expect(overlayRef.hostElement.parentElement)
+        .toBeFalsy('Expected host element to have been removed once the zone stabilizes.');
+
+    overlayRef.attach(componentPortal);
+    viewContainerFixture.detectChanges();
+
+    expect(overlayRef.hostElement.parentElement)
+        .toBeTruthy('Expected host element to be back in the DOM.');
+  });
+
   describe('positioning', () => {
     let config: OverlayConfig;
 
@@ -354,6 +389,7 @@ describe('Overlay', () => {
 
       overlay.create(config).attach(componentPortal);
       viewContainerFixture.detectChanges();
+      zone.simulateZoneExit();
       tick();
 
       expect(overlayContainerElement.querySelectorAll('.fake-positioned').length).toBe(1);

src/lib/tooltip/tooltip.spec.ts

@@ -512,21 +512,24 @@ describe('MatTooltip', () => {
     it('should keep the overlay direction in sync with the trigger direction', fakeAsync(() => {
       dir.value = 'rtl';
       tooltipDirective.show();
-      tick();
+      tick(0);
       fixture.detectChanges();
+      tick(500);
 
       let tooltipWrapper =
           overlayContainerElement.querySelector('.cdk-overlay-connected-position-bounding-box')!;
       expect(tooltipWrapper.getAttribute('dir')).toBe('rtl', 'Expected tooltip to be in RTL.');
 
       tooltipDirective.hide(0);
-      tick();
+      tick(0);
       fixture.detectChanges();
+      tick(500);
 
       dir.value = 'ltr';
       tooltipDirective.show();
-      tick();
+      tick(0);
       fixture.detectChanges();
+      tick(500);
 
       tooltipWrapper =
           overlayContainerElement.querySelector('.cdk-overlay-connected-position-bounding-box')!;