From ff7b625851cf4c23b8e02f1ed1186448fa645601 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Fri, 5 Dec 2025 13:06:03 +0100 Subject: [PATCH] [CL-946] Migrate ToggleGroup to OnPush (#17718) Migrates the ToggleGroup and Toggle components to use OnPush. --- .../toggle-group.component.spec.ts | 19 +- .../toggle-group/toggle-group.component.ts | 39 ++-- .../src/toggle-group/toggle.component.html | 12 +- .../src/toggle-group/toggle.component.spec.ts | 64 ++++++- .../src/toggle-group/toggle.component.ts | 166 +++++++++--------- 5 files changed, 178 insertions(+), 122 deletions(-) diff --git a/libs/components/src/toggle-group/toggle-group.component.spec.ts b/libs/components/src/toggle-group/toggle-group.component.spec.ts index 3a9d35b862..5c1345df6f 100644 --- a/libs/components/src/toggle-group/toggle-group.component.spec.ts +++ b/libs/components/src/toggle-group/toggle-group.component.spec.ts @@ -1,4 +1,4 @@ -import { Component } from "@angular/core"; +import { ChangeDetectionStrategy, Component, signal, WritableSignal } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; @@ -30,31 +30,29 @@ describe("Button", () => { }); it("should select second element when setting selected to second", () => { - testAppComponent.selected = "second"; + testAppComponent.selected.set("second"); fixture.detectChanges(); - expect(buttonElements[1].selected).toBe(true); + expect(buttonElements[1].selected()).toBe(true); }); it("should not select second element when setting selected to third", () => { - testAppComponent.selected = "third"; + testAppComponent.selected.set("third"); fixture.detectChanges(); - expect(buttonElements[1].selected).toBe(false); + expect(buttonElements[1].selected()).toBe(false); }); it("should emit new value when changing selection by clicking on radio button", () => { - testAppComponent.selected = "first"; + testAppComponent.selected.set("first"); fixture.detectChanges(); radioButtons[1].click(); - expect(testAppComponent.selected).toBe("second"); + expect(testAppComponent.selected()).toBe("second"); }); }); -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "test-app", template: ` @@ -65,7 +63,8 @@ describe("Button", () => { `, imports: [ToggleGroupModule], + changeDetection: ChangeDetectionStrategy.OnPush, }) class TestAppComponent { - selected?: string; + readonly selected: WritableSignal = signal(undefined); } diff --git a/libs/components/src/toggle-group/toggle-group.component.ts b/libs/components/src/toggle-group/toggle-group.component.ts index bde8f0ea9b..ee619eb15d 100644 --- a/libs/components/src/toggle-group/toggle-group.component.ts +++ b/libs/components/src/toggle-group/toggle-group.component.ts @@ -1,39 +1,44 @@ import { booleanAttribute, + ChangeDetectionStrategy, Component, - EventEmitter, - HostBinding, - Output, + computed, input, model, } from "@angular/core"; let nextId = 0; -// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush -// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection @Component({ selector: "bit-toggle-group", templateUrl: "./toggle-group.component.html", + changeDetection: ChangeDetectionStrategy.OnPush, + host: { + role: "radiogroup", + "[class]": "classlist()", + }, }) export class ToggleGroupComponent { - private id = nextId++; - name = `bit-toggle-group-${this.id}`; + private readonly id = nextId++; + readonly name = `bit-toggle-group-${this.id}`; + + /** + * Whether the toggle group should take up the full width of its container. + * When true, each toggle button will be equally sized to fill the available space. + */ readonly fullWidth = input(undefined, { transform: booleanAttribute }); - readonly selected = model(); - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-output-emitter-ref - @Output() selectedChange = new EventEmitter(); - @HostBinding("attr.role") role = "radiogroup"; - @HostBinding("class") - get classList() { - return ["tw-flex"].concat(this.fullWidth() ? ["tw-w-full", "[&>*]:tw-flex-1"] : []); - } + /** + * The selected value in the toggle group. + */ + readonly selected = model(); + + protected readonly classlist = computed(() => + ["tw-flex"].concat(this.fullWidth() ? ["tw-w-full", "[&>*]:tw-flex-1"] : []), + ); onInputInteraction(value: TValue) { this.selected.set(value); - this.selectedChange.emit(value); } } diff --git a/libs/components/src/toggle-group/toggle.component.html b/libs/components/src/toggle-group/toggle.component.html index d036b1751b..5104fbb864 100644 --- a/libs/components/src/toggle-group/toggle.component.html +++ b/libs/components/src/toggle-group/toggle.component.html @@ -1,16 +1,16 @@ -