perf(stark-ui): set ChangeDetectionStrategy.OnPush to UI components t…

…o prevent Angular from running unnecessary change detection cycles

ISSUES CLOSED: NationalBankBelgium#1331

packages/stark-ui/src/modules/action-bar/components/action-bar.component.spec.ts

@@ -1,4 +1,5 @@
-import { NO_ERRORS_SCHEMA } from "@angular/core";
+/* tslint:disable:completed-docs max-inline-declarations */
+import { Component, NO_ERRORS_SCHEMA, ViewChild } from "@angular/core";
 import { async, ComponentFixture, TestBed } from "@angular/core/testing";
 import { MatButtonModule } from "@angular/material/button";
 import { MatMenuModule } from "@angular/material/menu";
@@ -8,18 +9,43 @@ import { STARK_LOGGING_SERVICE } from "@nationalbankbelgium/stark-core";
 import { MockStarkLoggingService } from "@nationalbankbelgium/stark-core/testing";
 import { TranslateModule, TranslateService } from "@ngx-translate/core";
 import { Subject } from "rxjs";
-import { StarkActionBarComponent } from "./action-bar.component";
+import { StarkActionBarComponent, StarkActionBarComponentMode } from "./action-bar.component";
 import { StarkAction } from "./action.intf";
+import { StarkActionBarConfig } from "./action-bar-config.intf";
 import createSpy = jasmine.createSpy;
 
 describe("ActionBarComponent", () => {
-	let fixture: ComponentFixture<StarkActionBarComponent>;
+	@Component({
+		selector: "host-component",
+		template: `
+			<stark-action-bar
+				[mode]="mode"
+				[actionBarId]="actionBarId"
+				[actionBarConfig]="actionBarConfig"
+				[alternativeActions]="alternativeActions"
+			></stark-action-bar>
+		`
+	})
+	class TestHostComponent {
+		@ViewChild(StarkActionBarComponent)
+		public starkActionBar!: StarkActionBarComponent;
+
+		public mode?: StarkActionBarComponentMode;
+		public actionBarId?: string;
+		public actionBarConfig: StarkActionBarConfig = { actions: [] };
+		public alternativeActions?: StarkAction[];
+	}
+
+	// IMPORTANT: The official way to test components using ChangeDetectionStrategy.OnPush is to wrap it with a test host component
+	// see https://github.com/angular/angular/issues/12313#issuecomment-444623173
+	let hostFixture: ComponentFixture<TestHostComponent>;
+	let hostComponent: TestHostComponent;
 	let component: StarkActionBarComponent;
 	const buttonToggleSelector = ".extend-action-bar";
 
 	beforeEach(async(() => {
 		return TestBed.configureTestingModule({
-			declarations: [StarkActionBarComponent],
+			declarations: [StarkActionBarComponent, TestHostComponent],
 			imports: [MatButtonModule, MatMenuModule, MatTooltipModule, TranslateModule.forRoot()],
 			providers: [
 				{ provide: STARK_LOGGING_SERVICE, useValue: new MockStarkLoggingService() },
@@ -35,8 +61,8 @@ describe("ActionBarComponent", () => {
 	}));
 
 	beforeEach(() => {
-		fixture = TestBed.createComponent(StarkActionBarComponent);
-		component = fixture.componentInstance;
+		hostFixture = TestBed.createComponent(TestHostComponent);
+		hostComponent = hostFixture.componentInstance;
 		const demoActions: StarkAction[] = [
 			{
 				id: "userDetailValidate",
@@ -56,78 +82,87 @@ describe("ActionBarComponent", () => {
 			}
 		];
 
-		component.actionBarConfig = {
+		hostComponent.actionBarConfig = {
 			actions: demoActions,
 			isPresent: true
 		};
-		fixture.detectChanges();
+		hostFixture.detectChanges();
+		component = hostComponent.starkActionBar;
 	});
 
 	describe("@Input() mode", () => {
 		it("should have the toggle action bar button visible in full mode", () => {
-			component.mode = "full";
-			fixture.detectChanges();
-			const buttonToggleExtend: HTMLElement = fixture.nativeElement.querySelector(buttonToggleSelector);
+			hostComponent.mode = "full";
+			hostFixture.detectChanges();
+
+			const buttonToggleExtend: HTMLElement = hostFixture.nativeElement.querySelector(buttonToggleSelector);
 			expect(buttonToggleExtend).toBeDefined();
 		});
 
 		it("should not have the toggle action bar button visible in compact mode", () => {
-			component.mode = "compact";
-			fixture.detectChanges();
-			const buttonToggleExtend: HTMLElement = fixture.nativeElement.querySelector(buttonToggleSelector);
+			hostComponent.mode = "compact";
+			hostFixture.detectChanges();
+
+			const buttonToggleExtend: HTMLElement = hostFixture.nativeElement.querySelector(buttonToggleSelector);
 			expect(buttonToggleExtend).toBeNull();
 		});
 	});
 
 	describe("@Input() actionBarId", () => {
 		it("should have set the id of the action bar", () => {
-			component.actionBarId = "action-bar-id";
-			fixture.detectChanges();
-			const actionBar: HTMLElement = fixture.nativeElement.querySelector("#" + component.actionBarId);
+			hostComponent.actionBarId = "action-bar-id";
+			hostFixture.detectChanges();
+
+			const actionBar: HTMLElement = hostFixture.nativeElement.querySelector("#" + hostComponent.actionBarId);
 			expect(actionBar).toBeDefined();
 		});
 	});
 
 	describe("@Input() actionBarConfig", () => {
 		it("should not call the defined action when disabled", () => {
-			const menuItem: HTMLElement = fixture.nativeElement.querySelector(
-				"#" + component.actionBarId + "-" + component.actionBarConfig.actions[0].id
+			const menuItem: HTMLElement = hostFixture.nativeElement.querySelector(
+				"#" + hostComponent.actionBarId + "-" + hostComponent.actionBarConfig.actions[0].id
 			);
 			menuItem.click();
-			expect(component.actionBarConfig.actions[0].actionCall).not.toHaveBeenCalled();
+			expect(hostComponent.actionBarConfig.actions[0].actionCall).not.toHaveBeenCalled();
 		});
 
 		it("should call the defined action when enabled", () => {
-			const menuItem: HTMLElement = fixture.nativeElement.querySelector(
-				"#" + component.actionBarId + "-" + component.actionBarConfig.actions[1].id
+			const menuItem: HTMLElement = hostFixture.nativeElement.querySelector(
+				"#" + hostComponent.actionBarId + "-" + hostComponent.actionBarConfig.actions[1].id
 			);
 			menuItem.click();
-			expect(component.actionBarConfig.actions[1].actionCall).toHaveBeenCalledTimes(1);
+			expect(hostComponent.actionBarConfig.actions[1].actionCall).toHaveBeenCalledTimes(1);
 		});
 	});
 
 	describe("@Input() alternativeActions", () => {
 		beforeEach(() => {
-			component.alternativeActions = component.actionBarConfig.actions;
-			fixture.detectChanges();
+			hostComponent.alternativeActions = hostComponent.actionBarConfig.actions;
+			hostFixture.detectChanges();
 		});
 
 		it("should display", () => {
-			const actionBar: HTMLElement = fixture.nativeElement.querySelector(".open-alt-actions");
+			const actionBar: HTMLElement = hostFixture.nativeElement.querySelector(".open-alt-actions");
 			expect(actionBar).toBeDefined();
 		});
 	});
 
 	describe("toggle extended action bar", () => {
 		it("should toggle the action bar extension", () => {
-			const buttonToggleExtend: HTMLElement = fixture.nativeElement.querySelector(buttonToggleSelector);
+			hostComponent.mode = "full";
+			hostFixture.detectChanges();
+
+			const buttonToggleExtend: HTMLElement = hostFixture.nativeElement.querySelector(buttonToggleSelector);
 			spyOn(component, "toggleExtendedActionBar").and.callThrough();
 			buttonToggleExtend.click();
-			fixture.detectChanges();
+			hostFixture.detectChanges();
+
 			expect(component.toggleExtendedActionBar).toHaveBeenCalledTimes(1);
 			expect(component.isExtended).toBe(true);
 			buttonToggleExtend.click();
-			fixture.detectChanges();
+
+			hostFixture.detectChanges();
 			expect(component.toggleExtendedActionBar).toHaveBeenCalledTimes(2);
 			expect(component.isExtended).toBe(false);
 		});

packages/stark-ui/src/modules/action-bar/components/action-bar.component.ts

@@ -1,4 +1,4 @@
-import { Component, ElementRef, Inject, Input, OnInit, Renderer2, ViewEncapsulation } from "@angular/core";
+import { ChangeDetectionStrategy, Component, ElementRef, Inject, Input, OnInit, Renderer2, ViewEncapsulation } from "@angular/core";
 import { StarkActionBarConfig } from "./action-bar-config.intf";
 import { StarkAction, StarkActionBarButtonColor } from "./action.intf";
 import { STARK_LOGGING_SERVICE, StarkLoggingService } from "@nationalbankbelgium/stark-core";
@@ -18,6 +18,7 @@ const componentName = "stark-action-bar";
 	selector: "stark-action-bar",
 	templateUrl: "./action-bar.component.html",
 	encapsulation: ViewEncapsulation.None,
+	changeDetection: ChangeDetectionStrategy.OnPush,
 	// We need to use host instead of @HostBinding: https://github.com/NationalBankBelgium/stark/issues/664
 	host: {
 		class: componentName

packages/stark-ui/src/modules/app-footer/components/app-footer.component.ts

@@ -1,4 +1,4 @@
-import { Component, HostBinding, Inject, Input, OnInit } from "@angular/core";
+import { ChangeDetectionStrategy, Component, HostBinding, Inject, Input, OnInit } from "@angular/core";
 import { TranslateService } from "@ngx-translate/core";
 import { STARK_LOGGING_SERVICE, StarkLoggingService } from "@nationalbankbelgium/stark-core";
 
@@ -12,7 +12,8 @@ const componentName = "stark-app-footer";
  */
 @Component({
 	selector: "stark-app-footer",
-	templateUrl: "./app-footer.component.html"
+	templateUrl: "./app-footer.component.html",
+	changeDetection: ChangeDetectionStrategy.OnPush
 })
 export class StarkAppFooterComponent implements OnInit {
 	/**

packages/stark-ui/src/modules/app-logout/components/app-logout.component.ts

@@ -1,4 +1,14 @@
-import { Component, ElementRef, Inject, Input, OnInit, Optional, Renderer2, ViewEncapsulation } from "@angular/core";
+import {
+	ChangeDetectionStrategy,
+	Component,
+	ElementRef,
+	Inject,
+	Input,
+	OnInit,
+	Optional,
+	Renderer2,
+	ViewEncapsulation
+} from "@angular/core";
 
 import {
 	STARK_LOGGING_SERVICE,
@@ -25,6 +35,7 @@ const componentName = "stark-app-logout";
 	selector: "stark-app-logout",
 	templateUrl: "./app-logout.component.html",
 	encapsulation: ViewEncapsulation.None,
+	changeDetection: ChangeDetectionStrategy.OnPush,
 	// We need to use host instead of @HostBinding: https://github.com/NationalBankBelgium/stark/issues/664
 	host: {
 		class: componentName

packages/stark-ui/src/modules/app-menu/components/app-menu-item.component.spec.ts

@@ -1,17 +1,41 @@
 /*tslint:disable:completed-docs*/
 import { NoopAnimationsModule } from "@angular/platform-browser/animations";
 import { async, ComponentFixture, TestBed } from "@angular/core/testing";
-import { NO_ERRORS_SCHEMA } from "@angular/compiler/src/core";
+import { Component, NO_ERRORS_SCHEMA, ViewChild } from "@angular/core";
 import { UIRouterModule } from "@uirouter/angular";
 import { MatExpansionModule } from "@angular/material/expansion";
 import { MatListModule } from "@angular/material/list";
 import { STARK_LOGGING_SERVICE, STARK_ROUTING_SERVICE } from "@nationalbankbelgium/stark-core";
 import { MockStarkLoggingService, MockStarkRoutingService } from "@nationalbankbelgium/stark-core/testing";
 import { StarkAppMenuItemComponent } from "./app-menu-item.component";
+import { StarkMenuGroup } from "./app-menu-group.intf";
 
 describe("StarkAppMenuItemComponent", () => {
+	@Component({
+		selector: "host-component",
+		template: `
+			<stark-app-menu-item [level]="level" [menuGroup]="menuGroup"></stark-app-menu-item>
+		`
+	})
+	class TestHostComponent {
+		@ViewChild(StarkAppMenuItemComponent)
+		public starkAppMenuItem!: StarkAppMenuItemComponent;
+
+		public level?: number;
+		public menuGroup: StarkMenuGroup = {
+			id: "id-item",
+			label: "Label",
+			isVisible: true,
+			isEnabled: true,
+			targetState: "test"
+		};
+	}
+
+	// IMPORTANT: The official way to test components using ChangeDetectionStrategy.OnPush is to wrap it with a test host component
+	// see https://github.com/angular/angular/issues/12313#issuecomment-444623173
+	let hostComponent: TestHostComponent;
 	let component: StarkAppMenuItemComponent;
-	let fixture: ComponentFixture<StarkAppMenuItemComponent>;
+	let hostFixture: ComponentFixture<TestHostComponent>;
 	const mockRoutingService: MockStarkRoutingService = new MockStarkRoutingService();
 
 	/**
@@ -20,7 +44,7 @@ describe("StarkAppMenuItemComponent", () => {
 	beforeEach(async(() => {
 		return (
 			TestBed.configureTestingModule({
-				declarations: [StarkAppMenuItemComponent],
+				declarations: [StarkAppMenuItemComponent, TestHostComponent],
 				imports: [MatExpansionModule, MatListModule, NoopAnimationsModule, UIRouterModule],
 				providers: [
 					{ provide: STARK_LOGGING_SERVICE, useValue: new MockStarkLoggingService() },
@@ -39,72 +63,73 @@ describe("StarkAppMenuItemComponent", () => {
 	 * Synchronous beforeEach
 	 */
 	beforeEach(() => {
-		fixture = TestBed.createComponent(StarkAppMenuItemComponent);
-		component = fixture.componentInstance;
-		component.menuGroup = {
-			id: "id-item",
-			label: "Label",
-			isVisible: true,
-			isEnabled: true,
-			targetState: "test"
-		};
-		component.level = 1;
-		fixture.detectChanges();
+		hostFixture = TestBed.createComponent(TestHostComponent);
+		hostComponent = hostFixture.componentInstance;
+		hostComponent.level = 1;
+		hostFixture.detectChanges();
+		component = hostComponent.starkAppMenuItem;
 		mockRoutingService.navigateTo.calls.reset();
 	});
 
 	describe("Simple menu item", () => {
 		it("id should be set", () => {
-			const item: HTMLElement = fixture.nativeElement.querySelector("#" + component.menuGroup.id);
+			const item: HTMLElement = hostFixture.nativeElement.querySelector("#" + hostComponent.menuGroup.id);
 			expect(item).toBeDefined();
 		});
 
 		it("should navigate", () => {
-			const item: HTMLElement = fixture.nativeElement.querySelector("#" + component.menuGroup.id);
+			const item: HTMLElement = hostFixture.nativeElement.querySelector("#" + hostComponent.menuGroup.id);
 			item.click();
-			fixture.detectChanges();
+			hostFixture.detectChanges();
+
 			expect(mockRoutingService.navigateTo).toHaveBeenCalledTimes(1);
 		});
 
 		it("should be set to disabled", () => {
-			component.menuGroup.isEnabled = false;
-			const item: HTMLElement = fixture.nativeElement.querySelector("#" + component.menuGroup.id);
-			fixture.detectChanges();
+			hostComponent.menuGroup = { ...hostComponent.menuGroup, isEnabled: false };
+			hostFixture.detectChanges();
+
+			const item: HTMLElement = hostFixture.nativeElement.querySelector("#" + hostComponent.menuGroup.id);
 			item.click();
-			fixture.detectChanges();
+			hostFixture.detectChanges();
+
 			expect(item.classList).toContain("stark-disabled");
 			expect(mockRoutingService.navigateTo).toHaveBeenCalledTimes(0);
 		});
 
 		it("should be set to visible", () => {
-			component.menuGroup.isVisible = false;
-			fixture.detectChanges();
-			const item: HTMLElement = fixture.nativeElement.querySelector("#" + component.menuGroup.id);
+			hostComponent.menuGroup = { ...hostComponent.menuGroup, isVisible: false };
+			hostFixture.detectChanges();
+
+			const item: HTMLElement = hostFixture.nativeElement.querySelector("#" + hostComponent.menuGroup.id);
 			expect(item).toBeNull();
 
-			component.menuGroup.isVisible = true;
-			fixture.detectChanges();
+			hostComponent.menuGroup = { ...hostComponent.menuGroup, isVisible: true };
+			hostFixture.detectChanges();
+
 			expect(item).toBeDefined();
 		});
 
 		it("should dispatch activated event", () => {
 			spyOn(component.activated, "emit");
 			component.isActive = true;
-			fixture.detectChanges();
+			hostFixture.detectChanges();
+
 			expect(component.activated.emit).toHaveBeenCalledTimes(1);
 		});
 
 		it("should dispatch deactivated event", () => {
 			spyOn(component.deactivated, "emit");
 			component.isActive = false;
-			fixture.detectChanges();
+			hostFixture.detectChanges();
+
 			expect(component.deactivated.emit).toHaveBeenCalledTimes(1);
 		});
 	});
 
 	describe("Menu item with children", () => {
 		beforeEach(() => {
-			component.menuGroup.entries = [
+			hostComponent.menuGroup.entries = [
 				{
 					id: "id-child-1",
 					label: "Child 1",
@@ -120,11 +145,11 @@ describe("StarkAppMenuItemComponent", () => {
 					targetState: "child-2-route"
 				}
 			];
-			fixture.detectChanges();
+			hostFixture.detectChanges();
 		});
 
 		it("should have an expansion panel", () => {
-			const expansionPanel: HTMLElement = fixture.nativeElement.querySelector(".mat-expansion-panel");
+			const expansionPanel: HTMLElement = hostFixture.nativeElement.querySelector(".mat-expansion-panel");
 			expect(expansionPanel).toBeDefined();
 		});
 	});