From 23365262343bcb70eb7fd0a28236ace93fd9e2e1 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 27 Nov 2017 22:57:16 +0100 Subject: [PATCH] refactor: remove the OverlayContainer stubs from all of the tests Gets rid of all the manual stubs of the `OverlayContainer` in the tests and replaces them with getting the container through DI. This removes some boilerplate and should make things less prone to breaking if we make any changes to the container. --- .../overlay-keyboard-dispatcher.spec.ts | 8 ----- src/cdk/overlay/overlay.spec.ts | 18 +++++----- .../connected-position-strategy.spec.ts | 33 +++++++------------ src/lib/autocomplete/autocomplete.spec.ts | 22 +++++-------- src/lib/dialog/dialog.spec.ts | 27 +++++++++------ src/lib/menu/menu.spec.ts | 20 +++++------ src/lib/select/select.spec.ts | 22 ++++--------- src/lib/tooltip/tooltip.spec.ts | 14 ++++---- 8 files changed, 69 insertions(+), 95 deletions(-) diff --git a/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts b/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts index 58e38158ea1b..93fa2a99ec0f 100644 --- a/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts +++ b/src/cdk/overlay/keyboard/overlay-keyboard-dispatcher.spec.ts @@ -3,7 +3,6 @@ import {dispatchKeyboardEvent} from '@angular/cdk/testing'; import {ESCAPE} from '@angular/cdk/keycodes'; import {Component, NgModule} from '@angular/core'; import {Overlay} from '../overlay'; -import {OverlayContainer} from '../overlay-container'; import {OverlayModule} from '../index'; import {OverlayKeyboardDispatcher} from './overlay-keyboard-dispatcher'; import {ComponentPortal} from '@angular/cdk/portal'; @@ -12,17 +11,10 @@ import {ComponentPortal} from '@angular/cdk/portal'; describe('OverlayKeyboardDispatcher', () => { let keyboardDispatcher: OverlayKeyboardDispatcher; let overlay: Overlay; - let overlayContainerElement: HTMLElement; beforeEach(() => { TestBed.configureTestingModule({ imports: [OverlayModule, TestComponentModule], - providers: [ - {provide: OverlayContainer, useFactory: () => { - overlayContainerElement = document.createElement('div'); - return {getContainerElement: () => overlayContainerElement}; - }} - ], }); }); diff --git a/src/cdk/overlay/overlay.spec.ts b/src/cdk/overlay/overlay.spec.ts index 2ea9ae873fa6..01fc5400ed3a 100644 --- a/src/cdk/overlay/overlay.spec.ts +++ b/src/cdk/overlay/overlay.spec.ts @@ -22,23 +22,19 @@ describe('Overlay', () => { let componentPortal: ComponentPortal; let templatePortal: TemplatePortal; let overlayContainerElement: HTMLElement; + let overlayContainer: OverlayContainer; let viewContainerFixture: ComponentFixture; beforeEach(async(() => { TestBed.configureTestingModule({ - imports: [OverlayModule, PortalModule, OverlayTestModule], - providers: [{ - provide: OverlayContainer, - useFactory: () => { - overlayContainerElement = document.createElement('div'); - return {getContainerElement: () => overlayContainerElement}; - } - }] + imports: [OverlayModule, PortalModule, OverlayTestModule] }).compileComponents(); })); - beforeEach(inject([Overlay], (o: Overlay) => { + beforeEach(inject([Overlay, OverlayContainer], (o: Overlay, oc: OverlayContainer) => { overlay = o; + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); let fixture = TestBed.createComponent(TestComponentWithTemplatePortals); fixture.detectChanges(); @@ -47,6 +43,10 @@ describe('Overlay', () => { viewContainerFixture = fixture; })); + afterEach(() => { + overlayContainer.ngOnDestroy(); + }); + it('should load a component into an overlay', () => { let overlayRef = overlay.create(); overlayRef.attach(componentPortal); diff --git a/src/cdk/overlay/position/connected-position-strategy.spec.ts b/src/cdk/overlay/position/connected-position-strategy.spec.ts index d39924f41ccb..b08b6ea53005 100644 --- a/src/cdk/overlay/position/connected-position-strategy.spec.ts +++ b/src/cdk/overlay/position/connected-position-strategy.spec.ts @@ -3,10 +3,12 @@ import {TestBed, inject} from '@angular/core/testing'; import {OverlayPositionBuilder} from './overlay-position-builder'; import {CdkScrollable} from '@angular/cdk/scrolling'; import {Subscription} from 'rxjs/Subscription'; +import {ScrollDispatchModule} from '@angular/cdk/scrolling'; import { OverlayModule, Overlay, OverlayRef, + OverlayContainer, ConnectedPositionStrategy, ConnectedOverlayPositionChange, } from '../index'; @@ -22,15 +24,23 @@ const DEFAULT_WIDTH = 60; describe('ConnectedPositionStrategy', () => { let positionBuilder: OverlayPositionBuilder; + let overlayContainer: OverlayContainer; + let overlayContainerElement: HTMLElement; beforeEach(() => { - TestBed.configureTestingModule({imports: [OverlayModule]}); + TestBed.configureTestingModule({imports: [ScrollDispatchModule, OverlayModule]}); - inject([Overlay], (overlay: Overlay) => { + inject([Overlay, OverlayContainer], (overlay: Overlay, oc: OverlayContainer) => { positionBuilder = overlay.position(); + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); })(); }); + afterEach(() => { + overlayContainer.ngOnDestroy(); + }); + describe('with origin on document body', () => { const ORIGIN_HEIGHT = DEFAULT_HEIGHT; const ORIGIN_WIDTH = DEFAULT_WIDTH; @@ -39,7 +49,6 @@ describe('ConnectedPositionStrategy', () => { let originElement: HTMLElement; let overlayElement: HTMLElement; - let overlayContainerElement: HTMLElement; let strategy: ConnectedPositionStrategy; let fakeElementRef: ElementRef; @@ -50,17 +59,14 @@ describe('ConnectedPositionStrategy', () => { beforeEach(() => { // The origin and overlay elements need to be in the document body in order to have geometry. originElement = createPositionedBlockElement(); - overlayContainerElement = createOverlayContainer(); overlayElement = createPositionedBlockElement(); document.body.appendChild(originElement); - document.body.appendChild(overlayContainerElement); overlayContainerElement.appendChild(overlayElement); fakeElementRef = new ElementRef(originElement); }); afterEach(() => { document.body.removeChild(originElement); - document.body.removeChild(overlayContainerElement); // Reset the origin geometry after each test so we don't accidently keep state between tests. originRect = null; @@ -543,7 +549,6 @@ describe('ConnectedPositionStrategy', () => { describe('onPositionChange with scrollable view properties', () => { let overlayElement: HTMLElement; - let overlayContainerElement: HTMLElement; let strategy: ConnectedPositionStrategy; let scrollable: HTMLDivElement; @@ -553,9 +558,7 @@ describe('ConnectedPositionStrategy', () => { beforeEach(() => { // Set up the overlay - overlayContainerElement = createOverlayContainer(); overlayElement = createPositionedBlockElement(); - document.body.appendChild(overlayContainerElement); overlayContainerElement.appendChild(overlayElement); // Set up the origin @@ -584,7 +587,6 @@ describe('ConnectedPositionStrategy', () => { afterEach(() => { onPositionChangeSubscription.unsubscribe(); document.body.removeChild(scrollable); - document.body.removeChild(overlayContainerElement); }); it('should not have origin or overlay clipped or out of view without scroll', () => { @@ -646,24 +648,20 @@ describe('ConnectedPositionStrategy', () => { describe('positioning properties', () => { let originElement: HTMLElement; let overlayElement: HTMLElement; - let overlayContainerElement: HTMLElement; let strategy: ConnectedPositionStrategy; let fakeElementRef: ElementRef; beforeEach(() => { // The origin and overlay elements need to be in the document body in order to have geometry. originElement = createPositionedBlockElement(); - overlayContainerElement = createOverlayContainer(); overlayElement = createPositionedBlockElement(); document.body.appendChild(originElement); - document.body.appendChild(overlayContainerElement); overlayContainerElement.appendChild(overlayElement); fakeElementRef = new ElementRef(originElement); }); afterEach(() => { document.body.removeChild(originElement); - document.body.removeChild(overlayContainerElement); }); describe('in ltr', () => { @@ -771,13 +769,6 @@ function createBlockElement() { return element; } -/** Creates the wrapper for all of the overlays. */ -function createOverlayContainer() { - let element = document.createElement('div'); - element.classList.add('cdk-overlay-container'); - return element; -} - /** Creates an overflow container with a set height and width with margin. */ function createOverflowContainerElement() { let element = document.createElement('div'); diff --git a/src/lib/autocomplete/autocomplete.spec.ts b/src/lib/autocomplete/autocomplete.spec.ts index 7d5bc2abe538..96cdab2fdf35 100644 --- a/src/lib/autocomplete/autocomplete.spec.ts +++ b/src/lib/autocomplete/autocomplete.spec.ts @@ -19,7 +19,7 @@ import { ViewChild, ViewChildren, } from '@angular/core'; -import {async, ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; +import {async, ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing'; import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms'; import {MatOption} from '@angular/material/core'; import {MatFormField, MatFormFieldModule} from '@angular/material/form-field'; @@ -39,6 +39,7 @@ import { describe('MatAutocomplete', () => { + let overlayContainer: OverlayContainer; let overlayContainerElement: HTMLElement; let dir: Direction; let scrolledSubject = new Subject(); @@ -68,18 +69,6 @@ describe('MatAutocomplete', () => { AutocompleteWithSelectEvent, ], providers: [ - {provide: OverlayContainer, useFactory: () => { - overlayContainerElement = document.createElement('div'); - overlayContainerElement.classList.add('cdk-overlay-container'); - - document.body.appendChild(overlayContainerElement); - - // remove body padding to keep consistent cross-browser - document.body.style.padding = '0'; - document.body.style.margin = '0'; - - return {getContainerElement: () => overlayContainerElement}; - }}, {provide: Directionality, useFactory: () => ({value: dir})}, {provide: ScrollDispatcher, useFactory: () => ({ scrolled: () => scrolledSubject.asObservable() @@ -88,10 +77,15 @@ describe('MatAutocomplete', () => { }); TestBed.compileComponents(); + + inject([OverlayContainer], (oc: OverlayContainer) => { + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); + })(); })); afterEach(() => { - document.body.removeChild(overlayContainerElement); + overlayContainer.ngOnDestroy(); }); describe('panel toggling', () => { diff --git a/src/lib/dialog/dialog.spec.ts b/src/lib/dialog/dialog.spec.ts index 3027969b375a..100ce79b9417 100644 --- a/src/lib/dialog/dialog.spec.ts +++ b/src/lib/dialog/dialog.spec.ts @@ -32,6 +32,7 @@ import {MAT_DIALOG_DATA, MatDialog, MatDialogModule, MatDialogRef} from './index describe('MatDialog', () => { let dialog: MatDialog; + let overlayContainer: OverlayContainer; let overlayContainerElement: HTMLElement; let testViewContainerRef: ViewContainerRef; @@ -42,10 +43,6 @@ describe('MatDialog', () => { TestBed.configureTestingModule({ imports: [MatDialogModule, DialogTestModule], providers: [ - {provide: OverlayContainer, useFactory: () => { - overlayContainerElement = document.createElement('div'); - return {getContainerElement: () => overlayContainerElement}; - }}, {provide: Location, useClass: SpyLocation} ], }); @@ -53,10 +50,17 @@ describe('MatDialog', () => { TestBed.compileComponents(); })); - beforeEach(inject([MatDialog, Location], (d: MatDialog, l: Location) => { - dialog = d; - mockLocation = l as SpyLocation; - })); + beforeEach(inject([MatDialog, Location, OverlayContainer], + (d: MatDialog, l: Location, oc: OverlayContainer) => { + dialog = d; + mockLocation = l as SpyLocation; + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); + })); + + afterEach(() => { + overlayContainer.ngOnDestroy(); + }); beforeEach(() => { viewContainerFixture = TestBed.createComponent(ComponentWithChildViewContainer); @@ -181,6 +185,9 @@ describe('MatDialog', () => { it('should close a dialog and get back a result before it is closed', fakeAsync(() => { const dialogRef = dialog.open(PizzaMsg, {viewContainerRef: testViewContainerRef}); + flush(); + viewContainerFixture.detectChanges(); + // beforeClose should emit before dialog container is destroyed const beforeCloseHandler = jasmine.createSpy('beforeClose callback').and.callFake(() => { expect(overlayContainerElement.querySelector('mat-dialog-container')) @@ -188,11 +195,11 @@ describe('MatDialog', () => { }); dialogRef.beforeClose().subscribe(beforeCloseHandler); - dialogRef.close('Bulbasaurus'); + dialogRef.close('Bulbasaur'); viewContainerFixture.detectChanges(); flush(); - expect(beforeCloseHandler).toHaveBeenCalledWith('Bulbasaurus'); + expect(beforeCloseHandler).toHaveBeenCalledWith('Bulbasaur'); expect(overlayContainerElement.querySelector('mat-dialog-container')).toBeNull(); })); diff --git a/src/lib/menu/menu.spec.ts b/src/lib/menu/menu.spec.ts index 96ceede4320a..e9918a1edd57 100644 --- a/src/lib/menu/menu.spec.ts +++ b/src/lib/menu/menu.spec.ts @@ -1,4 +1,4 @@ -import {async, ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; +import {async, ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {NoopAnimationsModule} from '@angular/platform-browser/animations'; import { @@ -38,6 +38,7 @@ import { describe('MatMenu', () => { + let overlayContainer: OverlayContainer; let overlayContainerElement: HTMLElement; let dir: Direction; @@ -57,25 +58,20 @@ describe('MatMenu', () => { FakeIcon ], providers: [ - {provide: OverlayContainer, useFactory: () => { - overlayContainerElement = document.createElement('div'); - overlayContainerElement.classList.add('cdk-overlay-container'); - document.body.appendChild(overlayContainerElement); - - // remove body padding to keep consistent cross-browser - document.body.style.padding = '0'; - document.body.style.margin = '0'; - return {getContainerElement: () => overlayContainerElement}; - }}, {provide: Directionality, useFactory: () => ({value: dir})} ] }); TestBed.compileComponents(); + + inject([OverlayContainer], (oc: OverlayContainer) => { + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); + })(); })); afterEach(() => { - document.body.removeChild(overlayContainerElement); + overlayContainer.ngOnDestroy(); }); it('should open the menu as an idempotent operation', () => { diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index f4b0d8c79114..e05221064f92 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -64,6 +64,7 @@ const platform = new Platform(); describe('MatSelect', () => { + let overlayContainer: OverlayContainer; let overlayContainerElement: HTMLElement; let dir: {value: 'ltr'|'rtl'}; let scrolledSubject = new Subject(); @@ -71,20 +72,6 @@ describe('MatSelect', () => { // Providers used for all mat-select tests const commonProviders = [ - { - provide: OverlayContainer, useFactory: () => { - overlayContainerElement = document.createElement('div') as HTMLElement; - overlayContainerElement.classList.add('cdk-overlay-container'); - - document.body.appendChild(overlayContainerElement); - - // remove body padding to keep consistent cross-browser - document.body.style.padding = '0'; - document.body.style.margin = '0'; - - return {getContainerElement: () => overlayContainerElement}; - }, - }, {provide: Directionality, useFactory: () => dir = {value: 'ltr'}}, { provide: ScrollDispatcher, useFactory: () => ({ @@ -114,10 +101,15 @@ describe('MatSelect', () => { declarations: declarations, providers: commonProviders, }).compileComponents(); + + inject([OverlayContainer], (oc: OverlayContainer) => { + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); + })(); } afterEach(() => { - document.body.removeChild(overlayContainerElement); + overlayContainer.ngOnDestroy(); }); describe('core', () => { diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index 1c2b481eec63..116640c1be57 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -3,6 +3,7 @@ import { ComponentFixture, fakeAsync, flushMicrotasks, + inject, TestBed, tick } from '@angular/core/testing'; @@ -33,6 +34,7 @@ import { const initialTooltipMessage = 'initial tooltip message'; describe('MatTooltip', () => { + let overlayContainer: OverlayContainer; let overlayContainerElement: HTMLElement; let dir: {value: Direction}; @@ -47,11 +49,6 @@ describe('MatTooltip', () => { ], providers: [ {provide: Platform, useValue: {IOS: false, isBrowser: true}}, - {provide: OverlayContainer, useFactory: () => { - overlayContainerElement = document.createElement('div'); - document.body.appendChild(overlayContainerElement); - return {getContainerElement: () => overlayContainerElement}; - }}, {provide: Directionality, useFactory: () => { return dir = {value: 'ltr'}; }} @@ -59,10 +56,15 @@ describe('MatTooltip', () => { }); TestBed.compileComponents(); + + inject([OverlayContainer], (oc: OverlayContainer) => { + overlayContainer = oc; + overlayContainerElement = oc.getContainerElement(); + })(); })); afterEach(() => { - document.body.removeChild(overlayContainerElement); + overlayContainer.ngOnDestroy(); }); describe('basic usage', () => {