fix(stark-ui): fix regression in ProgressIndicator directive introduc…

…ed in #1218

packages/stark-ui/src/modules/progress-indicator/components/progress-indicator.component.html

@@ -1 +1 @@
-<div class="stark-loading-icon" *ngIf="isShown"></div>
+<div class="stark-loading-icon"></div>

packages/stark-ui/src/modules/progress-indicator/components/progress-indicator.component.spec.ts

@@ -33,15 +33,10 @@ describe("ProgressIndicatorComponent", () => {
 		});
 	});
 
-	describe("display Progress Indicator", () => {
-		it("should not display the progress indicator if isShown is set to false", () => {
-			component.isShown = false;
-			expect(hostFixture.debugElement.query(By.css(".div"))).toBeNull();
-		});
-
-		it("should display the progress indicator if isShown is set to true", () => {
-			component.isShown = true;
-			expect(hostFixture.debugElement.query(By.css(".div"))).toBeDefined();
+	describe("progress indicator", () => {
+		it("should be correctly displayed", () => {
+			const progressIndicator = hostFixture.debugElement.query(By.css("div.stark-loading-icon"));
+			expect(progressIndicator).toBeTruthy();
 		});
 	});
 });

packages/stark-ui/src/modules/progress-indicator/components/progress-indicator.component.ts

@@ -9,7 +9,7 @@ import { AbstractStarkUiComponent } from "../../../common/classes/abstract-compo
 const componentName = "stark-progress-indicator";
 
 /**
- * Component that is dynamically created by the ProgressIndicatorDirective
+ * Component that is dynamically created by the {@link StarkProgressIndicatorDirective}
  */
 @Component({
 	selector: "stark-progress-indicator",
@@ -21,8 +21,6 @@ const componentName = "stark-progress-indicator";
 	}
 })
 export class StarkProgressIndicatorComponent extends AbstractStarkUiComponent implements OnInit {
-	public isShown = false;
-
 	public constructor(
 		@Inject(STARK_LOGGING_SERVICE) public logger: StarkLoggingService,
 		protected renderer: Renderer2,

packages/stark-ui/src/modules/progress-indicator/directives/progress-indicator.directive.spec.ts

@@ -1,7 +1,8 @@
 // tslint:disable:completed-docs
-import { Observable } from "rxjs";
-import { Component } from "@angular/core";
+import { Observable, of, Subject } from "rxjs";
+import { Component, ViewChild } from "@angular/core";
 import { async, ComponentFixture, TestBed } from "@angular/core/testing";
+import { By } from "@angular/platform-browser";
 import { BrowserDynamicTestingModule } from "@angular/platform-browser-dynamic/testing";
 import { STARK_LOGGING_SERVICE } from "@nationalbankbelgium/stark-core";
 import { MockStarkLoggingService } from "@nationalbankbelgium/stark-core/testing";
@@ -17,20 +18,27 @@ import { StarkProgressIndicatorComponent } from "../components/progress-indicato
 @Component({
 	selector: "test-component",
 	template: `
-		<div [starkProgressIndicator]="starkProgressIndicatorConfig"></div>
+		<div id="host-element" [starkProgressIndicator]="starkProgressIndicatorConfig"></div>
 	`
 })
 class TestComponent {
 	public starkProgressIndicatorConfig?: StarkProgressIndicatorConfig;
+
+	@ViewChild(StarkProgressIndicatorDirective)
+	public progressIndicatorDirective!: StarkProgressIndicatorDirective;
 }
 
 describe("StarkProgressIndicator", () => {
 	let hostFixture: ComponentFixture<TestComponent>;
 	let hostComponent: TestComponent;
+	const hostElementSelector = "#host-element";
+	const mockConfig: StarkProgressIndicatorConfig = { topic: "some-topic", type: StarkProgressIndicatorType.SPINNER };
 
-	const mockStarkProgressIndicatorService = new MockStarkProgressIndicatorService();
+	let mockStarkProgressIndicatorService!: MockStarkProgressIndicatorService;
 
 	beforeEach(async(() => {
+		mockStarkProgressIndicatorService = new MockStarkProgressIndicatorService();
+
 		return TestBed.configureTestingModule({
 			imports: [],
 			declarations: [TestComponent, StarkProgressIndicatorComponent, StarkProgressIndicatorDirective],
@@ -54,16 +62,103 @@ describe("StarkProgressIndicator", () => {
 		hostComponent = hostFixture.componentInstance;
 	});
 
-	it("should throw error when StarkProgressIndicatorConfig is not set", () => {
-		hostComponent.starkProgressIndicatorConfig = undefined;
+	describe("on initialization", () => {
+		describe("failure", () => {
+			it("should throw error when StarkProgressIndicatorConfig is not set", () => {
+				hostComponent.starkProgressIndicatorConfig = undefined;
+
+				expect(() => hostFixture.detectChanges()).toThrowError(
+					"StarkProgressIndicatorDirective: a StarkProgressIndicatorConfig is required."
+				);
+			});
+		});
+
+		describe("success", () => {
+			beforeEach(() => {
+				mockStarkProgressIndicatorService.isVisible.and.returnValue(of(false));
+				hostComponent.starkProgressIndicatorConfig = mockConfig;
+
+				hostFixture.detectChanges();
+			});
+
+			it("should register itself to the service", () => {
+				expect(mockStarkProgressIndicatorService.register).toHaveBeenCalledTimes(1);
+				expect(mockStarkProgressIndicatorService.register).toHaveBeenCalledWith(
+					mockConfig.topic,
+					StarkProgressIndicatorType[mockConfig.type]
+				);
+			});
+
+			it("should subscribe to the service to be notified when the indicator for the given topic should be visible/hidden", () => {
+				expect(mockStarkProgressIndicatorService.isVisible).toHaveBeenCalledTimes(1);
+				expect(mockStarkProgressIndicatorService.isVisible).toHaveBeenCalledWith(mockConfig.topic);
+			});
+		});
+	});
+
+	describe("show/hide", () => {
+		let isVisible$: Subject<boolean>;
+
+		beforeEach(() => {
+			isVisible$ = new Subject();
+			mockStarkProgressIndicatorService.isVisible.and.returnValue(isVisible$);
+			hostComponent.starkProgressIndicatorConfig = mockConfig;
+
+			hostFixture.detectChanges();
+		});
+
+		it("should show/hide the host element and simultaneously hide/show the progress indicator", () => {
+			expect(mockStarkProgressIndicatorService.register).toHaveBeenCalledTimes(1);
 
-		expect(() => hostFixture.detectChanges()).toThrowError(
-			"StarkProgressIndicatorDirective: a StarkProgressIndicatorConfig is required."
-		);
+			let progressIndicatorComponent = hostFixture.debugElement.query(By.directive(StarkProgressIndicatorComponent));
+			const hostElement = hostFixture.debugElement.query(By.css(hostElementSelector));
+
+			expect(hostElement).toBeTruthy();
+			expect(hostElement.classes).toEqual({}); // host element should be shown
+			expect(progressIndicatorComponent).toBeFalsy(); // progress indicator should be hidden
+
+			isVisible$.next(true); // show
+			progressIndicatorComponent = hostFixture.debugElement.query(By.directive(StarkProgressIndicatorComponent));
+
+			expect(hostElement.classes).toEqual({ "stark-hide": true }); // host element should be hidden
+			expect(progressIndicatorComponent).toBeTruthy(); // progress indicator should be shown
+
+			isVisible$.next(false); // hide again
+			progressIndicatorComponent = hostFixture.debugElement.query(By.directive(StarkProgressIndicatorComponent));
+
+			expect(hostElement.classes).toEqual({ "stark-hide": false }); // host element should be shown
+			expect(progressIndicatorComponent).toBeFalsy(); // progress indicator should be hidden
+		});
 	});
 
-	it("should not throw error when StarkProgressIndicatorConfig is set", () => {
-		hostComponent.starkProgressIndicatorConfig = { topic: "something", type: StarkProgressIndicatorType.SPINNER };
-		expect(() => hostFixture.detectChanges()).not.toThrowError();
+	describe("ngOnDestroy", () => {
+		beforeEach(() => {
+			mockStarkProgressIndicatorService.isVisible.and.returnValue(of(true));
+			hostComponent.starkProgressIndicatorConfig = mockConfig;
+
+			hostFixture.detectChanges();
+		});
+
+		it("should destroy the progress indicator and leave only the host element", () => {
+			let progressIndicatorComponent = hostFixture.debugElement.query(By.directive(StarkProgressIndicatorComponent));
+			let hostElement = hostFixture.debugElement.query(By.css(hostElementSelector));
+
+			expect(hostElement).toBeTruthy();
+			expect(progressIndicatorComponent).toBeTruthy(); // progress indicator should be shown
+
+			hostComponent.progressIndicatorDirective.ngOnDestroy();
+
+			hostElement = hostFixture.debugElement.query(By.css(hostElementSelector));
+			expect(hostElement).toBeTruthy();
+			progressIndicatorComponent = hostFixture.debugElement.query(By.directive(StarkProgressIndicatorComponent));
+			expect(progressIndicatorComponent).toBeFalsy(); // progress indicator should be removed
+		});
+
+		it("should de-register itself from the service", () => {
+			hostComponent.progressIndicatorDirective.ngOnDestroy();
+
+			expect(mockStarkProgressIndicatorService.deregister).toHaveBeenCalledTimes(1);
+			expect(mockStarkProgressIndicatorService.deregister).toHaveBeenCalledWith(mockConfig.topic);
+		});
 	});
 });

packages/stark-ui/src/modules/progress-indicator/directives/progress-indicator.directive.ts

@@ -1,15 +1,15 @@
 import {
-	ComponentFactory,
 	ComponentFactoryResolver,
-	ComponentRef,
 	Directive,
 	ElementRef,
 	Inject,
+	Injector,
 	Input,
 	OnDestroy,
 	OnInit,
 	Renderer2,
-	ViewContainerRef
+	ViewContainerRef,
+	ViewRef
 } from "@angular/core";
 import { STARK_PROGRESS_INDICATOR_SERVICE, StarkProgressIndicatorService } from "../services";
 import { StarkProgressIndicatorConfig, StarkProgressIndicatorType } from "../entities";
@@ -53,18 +53,20 @@ export class StarkProgressIndicatorDirective implements OnInit, OnDestroy {
 
 	public topic!: string;
 	public type!: StarkProgressIndicatorType | string;
-	public progressSubscription!: Subscription;
-	private _componentFactory: ComponentFactory<StarkProgressIndicatorComponent>;
-	public _componentRef!: ComponentRef<StarkProgressIndicatorComponent>;
+	public progressSubscription?: Subscription;
+	private readonly componentViewRef!: ViewRef;
 
 	public constructor(
 		@Inject(STARK_PROGRESS_INDICATOR_SERVICE) public _progressService: StarkProgressIndicatorService,
 		componentFactoryResolver: ComponentFactoryResolver,
+		injector: Injector,
 		private _viewContainer: ViewContainerRef,
 		protected renderer: Renderer2,
 		protected elementRef: ElementRef
 	) {
-		this._componentFactory = componentFactoryResolver.resolveComponentFactory(StarkProgressIndicatorComponent);
+		const componentFactory = componentFactoryResolver.resolveComponentFactory(StarkProgressIndicatorComponent);
+		const componentRef = componentFactory.create(injector);
+		this.componentViewRef = componentRef.hostView;
 	}
 
 	/**
@@ -83,41 +85,40 @@ export class StarkProgressIndicatorDirective implements OnInit, OnDestroy {
 	}
 
 	/**
-	 * The directive registers itself with the StarkProgressIndicator service.
+	 * The directive registers itself to the {@link StarkProgressIndicatorService}.
 	 * The component to add is then created and inserted inside of the container.
-	 * Finally, if the component should be hidden or shown, the stark-hide class is removed/added accordingly
+	 * Finally, if the host component should be hidden or shown, the "stark-hide" class is removed/added accordingly
 	 */
 	public ngOnInit(): void {
-		this._componentRef = this._viewContainer.createComponent(this._componentFactory);
-
-		// TODO The element is here added as a child, not as a sibling
-		// this.renderer.appendChild(this.elementRef.nativeElement, this._componentRef.location.nativeElement);
-
-		this._viewContainer.insert(this._componentRef.hostView);
+		if (!this.starkProgressIndicator) {
+			throw new Error("StarkProgressIndicatorDirective: a StarkProgressIndicatorConfig is required.");
+		}
+		this.registerInstance(this.starkProgressIndicator);
 
 		this.progressSubscription = this._progressService.isVisible(this.topic).subscribe(
 			(isVisible: boolean = false): void => {
-				this._componentRef.instance.isShown = isVisible;
 				if (isVisible) {
+					// TODO The element is here added as a child, not as a sibling
+					// this.renderer.appendChild(this.elementRef.nativeElement, componentRef.location.nativeElement);
+
+					this._viewContainer.insert(this.componentViewRef); // insert the view in the last position
 					this.renderer.addClass(this.elementRef.nativeElement, "stark-hide");
 				} else {
+					this._viewContainer.detach(this._viewContainer.indexOf(this.componentViewRef));
 					this.renderer.removeClass(this.elementRef.nativeElement, "stark-hide");
 				}
 			}
 		);
-
-		if (!this.starkProgressIndicator) {
-			throw new Error("StarkProgressIndicatorDirective: a StarkProgressIndicatorConfig is required.");
-		}
-		this.registerInstance(this.starkProgressIndicator);
 	}
 
 	/**
-	 * The directive de-registers itself from the StarkProgressIndicator service when it is destroyed.
+	 * The directive de-registers itself from the {@link StarkProgressIndicatorService} when it is destroyed.
 	 */
 	public ngOnDestroy(): void {
-		this._viewContainer.clear();
-		this.progressSubscription.unsubscribe();
+		this.componentViewRef.destroy(); // destroy the progress indicator
+		if (this.progressSubscription) {
+			this.progressSubscription.unsubscribe();
+		}
 		this._progressService.deregister(this.topic);
 	}
 }