Skip to content

Commit

Permalink
feat: convert components to use OnPush change detection strategy (#3094)
Browse files Browse the repository at this point in the history
Signed-off-by: rrothschild18 <[email protected]>
Co-authored-by: Akshat Patel <[email protected]>
  • Loading branch information
Rrothschild18 and Akshat55 authored Feb 3, 2025
1 parent b2368b7 commit 0b26a53
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 44 deletions.
2 changes: 1 addition & 1 deletion src/accordion/accordion-item.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import {
display: list-item;
}
`],
changeDetection: ChangeDetectionStrategy.OnPush
changeDetection: ChangeDetectionStrategy.OnPush
})
export class AccordionItem {
static accordionItemCount = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/breadcrumb/breadcrumb-item.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { Router } from "@angular/router";
<ng-container *ngTemplateOutlet="content" />
}
`,
changeDetection: ChangeDetectionStrategy.OnPush
changeDetection: ChangeDetectionStrategy.OnPush
})
export class BreadcrumbItemComponent {
@Input() set href(v: string) {
Expand Down
2 changes: 1 addition & 1 deletion src/breadcrumb/breadcrumb.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ const MINIMUM_OVERFLOW_THRESHOLD = 4;
}
</nav>
`,
changeDetection: ChangeDetectionStrategy.OnPush
changeDetection: ChangeDetectionStrategy.OnPush
})
export class Breadcrumb implements AfterContentInit {
@ContentChildren(BreadcrumbItemComponent) children: QueryList<BreadcrumbItemComponent>;
Expand Down
10 changes: 8 additions & 2 deletions src/loading/loading.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { Component, Input, HostBinding } from "@angular/core";
import {
ChangeDetectionStrategy,
Component,
HostBinding,
Input
} from "@angular/core";
import { I18n } from "carbon-components-angular/i18n";

/**
Expand All @@ -25,7 +30,8 @@ import { I18n } from "carbon-components-angular/i18n";
<circle class="cds--loading__stroke" cx="50%" cy="50%" r="44" />
</svg>
</div>
`
`,
changeDetection: ChangeDetectionStrategy.OnPush
})
export class Loading {
/**
Expand Down
10 changes: 5 additions & 5 deletions src/number-input/number.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { ComponentFixture, TestBed } from "@angular/core/testing";
import { By } from "@angular/platform-browser";

import { Number } from "./number.component";
import { FormsModule } from "@angular/forms";
import { I18nModule } from "../i18n/index";
import { IconModule } from "../icon/index";
import { Number } from "./number.component";

describe("Number", () => {
let component: Number;
Expand Down Expand Up @@ -71,13 +71,13 @@ describe("Number", () => {
});

it("should bind input label", () => {
component.label = "Number Input";
fixture.componentRef.setInput("label", "Number Input");
fixture.detectChanges();
labelElement = fixture.debugElement.query(By.css(".cds--label")).nativeElement;
expect(labelElement.innerHTML.includes("Number Input")).toEqual(true);
expect(containerElement.className.includes("cds--number--nolabel")).toEqual(false);

component.label = null;
fixture.componentRef.setInput("label", null);
fixture.detectChanges();
expect(fixture.debugElement.query(By.css(".cds--label"))).toBeNull();
expect(containerElement.className.includes("cds--number--nolabel")).toEqual(true);
Expand Down Expand Up @@ -186,10 +186,10 @@ describe("Number", () => {
});

it("should have dark and light theme", () => {
component.theme = "dark";
fixture.componentRef.setInput("theme", "dark");
fixture.detectChanges();
expect(containerElement.className.includes("cds--number--light")).toEqual(false);
component.theme = "light";
fixture.componentRef.setInput("theme", "light");
fixture.detectChanges();
expect(containerElement.className.includes("cds--number--light")).toEqual(true);
});
Expand Down
12 changes: 7 additions & 5 deletions src/number-input/number.component.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {
ChangeDetectionStrategy,
Component,
Input,
HostBinding,
EventEmitter,
HostBinding,
HostListener,
Input,
Output,
TemplateRef,
HostListener
TemplateRef
} from "@angular/core";
import { ControlValueAccessor, NG_VALUE_ACCESSOR } from "@angular/forms";

Expand Down Expand Up @@ -167,7 +168,8 @@ export class NumberChange {
useExisting: NumberComponent,
multi: true
}
]
],
changeDetection: ChangeDetectionStrategy.OnPush
})
export class NumberComponent implements ControlValueAccessor {
/**
Expand Down
8 changes: 5 additions & 3 deletions src/progress-indicator/progress-indicator.component.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {
ChangeDetectionStrategy,
Component,
EventEmitter,
Input,
Output,
EventEmitter
Output
} from "@angular/core";
import { I18n } from "carbon-components-angular/i18n";
import { Step } from "./progress-indicator-step.interface";
Expand Down Expand Up @@ -72,7 +73,8 @@ import { Step } from "./progress-indicator-step.interface";
</li>
}
</ul>
`
`,
changeDetection: ChangeDetectionStrategy.OnPush
})
export class ProgressIndicator {
@Input() get current() {
Expand Down
40 changes: 20 additions & 20 deletions src/search/search.component.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { ComponentFixture, TestBed, waitForAsync } from "@angular/core/testing";
import { By } from "@angular/platform-browser";

import { Search } from "./search.component";
import { FormsModule } from "@angular/forms";
import { I18nModule } from "../i18n/index";
import { IconModule } from "../icon/index";
import { Search } from "./search.component";

describe("Search", () => {
let component: Search;
Expand Down Expand Up @@ -38,46 +38,46 @@ describe("Search", () => {
});

it("should bind input value", () => {
component.value = "Text";
fixture.componentRef.setInput("value", "Text");
fixture.detectChanges();
expect(inputElement.value).toEqual("Text");
});

it("should bind input disabled", () => {
expect(inputElement.disabled).toEqual(false);
component.disabled = true;
fixture.componentRef.setInput("disabled", true);
fixture.detectChanges();
expect(inputElement.disabled).toEqual(true);
});

it("should bind input required", () => {
component.required = true;
fixture.componentRef.setInput("required", true);
fixture.detectChanges();
expect(inputElement.required).toEqual(true);
});

it("should display component of the correct size", () => {
containerElement = fixture.debugElement.query(By.css(".cds--search")).nativeElement;
component.size = "lg";
fixture.componentRef.setInput("size", "lg");
fixture.detectChanges();
expect(containerElement.className.includes("cds--search--lg")).toEqual(true);
component.size = "sm";
fixture.componentRef.setInput("size", "sm");
fixture.detectChanges();
expect(containerElement.className.includes("cds--search--sm")).toEqual(true);
});

it("should display clear button", () => {
clearButtonElement = fixture.debugElement.query(By.css("button")).nativeElement;
component.value = "";
fixture.componentRef.setInput("value", "");
fixture.detectChanges();
expect(clearButtonElement.className.includes("cds--search-close--hidden")).toEqual(true);
component.value = "Text";
fixture.componentRef.setInput("value", "Text");
fixture.detectChanges();
expect(clearButtonElement.className.includes("cds--search-close--hidden")).toEqual(false);
});

it("should clear input when clear button is clicked", () => {
component.value = "Text";
fixture.componentRef.setInput("value", "Text");
fixture.detectChanges();
clearButtonElement = fixture.debugElement.query(By.css("button")).nativeElement;
clearButtonElement.click();
Expand All @@ -86,8 +86,8 @@ describe("Search", () => {
});

it("should clear the input when the clear button is clicked on the expandable component", () => {
component.expandable = true;
component.value = "TextToClear";
fixture.componentRef.setInput("expandable", true);
fixture.componentRef.setInput("value", "TextToClear");
fixture.detectChanges();
clearButtonElement = fixture.debugElement.query(By.css("button")).nativeElement;
clearButtonElement.click();
Expand All @@ -97,7 +97,7 @@ describe("Search", () => {

it("should clear the input when the escape key is pressed", () => {
clearButtonElement = fixture.debugElement.query(By.css("button")).nativeElement;
component.value = "TextToClear";
fixture.componentRef.setInput("value", "TextToClear");
fixture.detectChanges();
expect(clearButtonElement.className.includes("cds--search-close--hidden")).toEqual(false);
component.keyDown(new KeyboardEvent("keydown", {
Expand All @@ -109,10 +109,10 @@ describe("Search", () => {
});

it("should clear the input and keep the expanded state open when the escape key is pressed", () => {
component.expandable = true;
component.active = true;
fixture.componentRef.setInput("expandable", true);
fixture.componentRef.setInput("active", true);
containerElement = fixture.debugElement.query(By.css(".cds--search")).nativeElement;
component.value = "TextToClear";
fixture.componentRef.setInput("value", "TextToClear");
fixture.detectChanges();
expect(containerElement.className.includes("cds--search--expanded")).toEqual(true);
component.keyDown(new KeyboardEvent("keydown", {
Expand All @@ -124,10 +124,10 @@ describe("Search", () => {
});

it("should close the expandable search component when esc is pressed when content is empty", () => {
component.expandable = true;
component.active = true;
fixture.componentRef.setInput("expandable", true);
fixture.componentRef.setInput("active", true);
containerElement = fixture.debugElement.query(By.css(".cds--search")).nativeElement;
component.value = "";
fixture.componentRef.setInput("value", "");
fixture.detectChanges();
expect(containerElement.className.includes("cds--search--expanded")).toEqual(true);
component.keyDown(new KeyboardEvent("keydown", {
Expand All @@ -140,10 +140,10 @@ describe("Search", () => {

it("should have dark and light theme", () => {
containerElement = fixture.debugElement.query(By.css(".cds--search")).nativeElement;
component.theme = "dark";
fixture.componentRef.setInput("theme", "dark");
fixture.detectChanges();
expect(containerElement.className.includes("cds--search--light")).toEqual(false);
component.theme = "light";
fixture.componentRef.setInput("theme", "light");
fixture.detectChanges();
expect(containerElement.className.includes("cds--search--light")).toEqual(true);
});
Expand Down
21 changes: 15 additions & 6 deletions src/search/search.component.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import {
ChangeDetectionStrategy,
ChangeDetectorRef,
Component,
Input,
ElementRef,
EventEmitter,
Output,
HostBinding,
ElementRef,
HostListener,
Input,
Output,
ViewChild
} from "@angular/core";
import { NG_VALUE_ACCESSOR, ControlValueAccessor } from "@angular/forms";
import { ControlValueAccessor, NG_VALUE_ACCESSOR } from "@angular/forms";
import { I18n } from "carbon-components-angular/i18n";

/**
Expand All @@ -29,7 +31,8 @@ import { I18n } from "carbon-components-angular/i18n";
useExisting: Search,
multi: true
}
]
],
changeDetection: ChangeDetectionStrategy.OnPush
})
export class Search implements ControlValueAccessor {
/**
Expand Down Expand Up @@ -151,7 +154,11 @@ export class Search implements ControlValueAccessor {
* Creates an instance of `Search`.
* @param i18n The i18n translations.
*/
constructor(protected elementRef: ElementRef, protected i18n: I18n) {
constructor(
protected elementRef: ElementRef,
protected i18n: I18n,
protected changeDetectorRef: ChangeDetectorRef
) {
Search.searchCount++;
}

Expand Down Expand Up @@ -234,6 +241,7 @@ export class Search implements ControlValueAccessor {
if (this.value === "") {
this.active = false;
this.open.emit(this.active);
this.changeDetectorRef.markForCheck();
}
} else if (event.key === "Enter") {
this.openSearch();
Expand All @@ -243,6 +251,7 @@ export class Search implements ControlValueAccessor {
if (event.key === "Escape") {
if (this.value !== "") {
this.clearSearch();
this.changeDetectorRef.markForCheck();
}
}
}
Expand Down

0 comments on commit 0b26a53

Please sign in to comment.