Skip to content

Commit

Permalink
refactor: migrate to signal-based inputs & queries + output fns (#865)
Browse files Browse the repository at this point in the history
* refactor: migrate to signal-based inputs & queries + output fns

* refactor: revert not-so-easy-to-migrate components

* test: fix unit tests with mocked components or missing inputs

* chore: use the navigation tabs makeSut arg around
  • Loading branch information
davidlj95 authored Nov 27, 2024
1 parent da7705a commit 0090c07
Show file tree
Hide file tree
Showing 33 changed files with 80 additions and 66 deletions.
2 changes: 2 additions & 0 deletions src/app/common/simple-icon/simple-icon.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export class SimpleIconComponent {

protected _fillColor?: string

// TODO: Skipped for migration because:
// Accessor inputs cannot be migrated as they are too complex.
@Input({ required: true }) set icon(icon: SimpleIcon) {
this.loader(icon.slug)
.pipe(
Expand Down
6 changes: 3 additions & 3 deletions src/app/common/test-id.directive.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import { Directive, ElementRef, Input, OnChanges } from '@angular/core'
import { Directive, ElementRef, OnChanges, input } from '@angular/core'

@Directive({
selector: '[appTestId]',
standalone: true,
})
export class TestIdDirective implements OnChanges {
@Input() public appTestId!: string
public readonly appTestId = input.required<string>()

constructor(private el: ElementRef) {}

ngOnChanges(): void {
if (isDevMode) {
this.el.nativeElement.setAttribute(TEST_ID_ATTRIBUTE, this.appTestId)
this.el.nativeElement.setAttribute(TEST_ID_ATTRIBUTE, this.appTestId())
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<nav>
<app-tabs [selectedIndex]="_selectedIndex">
@for (item of items; track item.displayName) {
@for (item of items(); track item.displayName) {
<app-tab
[routerLink]="item.routerLink"
(isActiveChange)="$event ? onActiveRouteChange($index) : undefined"
Expand Down
16 changes: 8 additions & 8 deletions src/app/header/navigation-tabs/navigation-tabs.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ describe('NavigationTabsComponent', () => {
})

it('should display all items as tabs with their route links', () => {
const [fixture, component] = makeSut()
component.items = ITEMS
fixture.detectChanges()
const [fixture] = makeSut({ items: ITEMS })

const tabsElement = fixture.debugElement.query(byComponent(TabsComponent))

Expand All @@ -56,9 +54,7 @@ describe('NavigationTabsComponent', () => {
})

it('should mark only active page tab as selected', async () => {
const [fixture, component] = makeSut()
component.items = ITEMS
fixture.detectChanges()
const [fixture] = makeSut({ items: ITEMS })

await fixture.ngZone?.run(
async () => await RouterTestingHarness.create(FOO_ROUTE.path),
Expand All @@ -85,10 +81,14 @@ describe('NavigationTabsComponent', () => {
).toBeFalse()
})

const makeSut = () =>
componentTestSetup(NavigationTabsComponent, {
const makeSut = ({ items }: { items?: readonly NavigationItem[] } = {}) => {
const [fixture, component] = componentTestSetup(NavigationTabsComponent, {
providers: [provideRouter(ROUTES)],
})
fixture.componentRef.setInput('items', items ?? [])
fixture.detectChanges()
return [fixture, component] as const
}
})

function makeNavigationItemFromRoutePath(path: string): NavigationItem {
Expand Down
4 changes: 2 additions & 2 deletions src/app/header/navigation-tabs/navigation-tabs.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ChangeDetectionStrategy, Component, Input } from '@angular/core'
import { ChangeDetectionStrategy, Component, input } from '@angular/core'
import { TabComponent } from '../tab/tab.component'
import { TabsComponent } from '../tabs/tabs.component'
import { RouterLink, RouterLinkActive } from '@angular/router'
Expand All @@ -11,7 +11,7 @@ import { RouterLink, RouterLinkActive } from '@angular/router'
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class NavigationTabsComponent {
@Input({ required: true }) items!: readonly NavigationItem[]
readonly items = input.required<readonly NavigationItem[]>()
protected _selectedIndex?: number

onActiveRouteChange(index: number) {
Expand Down
4 changes: 2 additions & 2 deletions src/app/header/tab/tab.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('TabComponent', () => {

describe('when selected', () => {
beforeEach(() => {
component.selected = true
fixture.componentRef.setInput('selected', true)
fixture.detectChanges()
})

Expand All @@ -46,7 +46,7 @@ describe('TabComponent', () => {

describe('when not selected', () => {
beforeEach(() => {
component.selected = false
fixture.componentRef.setInput('selected', false)
fixture.detectChanges()
})

Expand Down
2 changes: 2 additions & 0 deletions src/app/header/tabs/tabs.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export class TabsComponent implements OnDestroy {
private _currentTabs: readonly TabComponent[] = []
private _indexToSelect?: number
private _selectedIndex?: number
// TODO: Skipped for migration because:
// Accessor inputs cannot be migrated as they are too complex.
@Input({ transform: numberAttribute }) set selectedIndex(index: number) {
this._indexToSelect = index
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<span appMaterialSymbol>{{ icon }}</span>
<span appMaterialSymbol>{{ icon() }}</span>
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ describe('ToolbarButtonComponent', () => {
const DUMMY_ICON = 'icon'
const makeSut = () => {
const [fixture, component] = componentTestSetup(ToolbarButtonComponent)
component.icon = DUMMY_ICON
fixture.componentRef.setInput('icon', DUMMY_ICON)
return [fixture, component] as const
}
5 changes: 3 additions & 2 deletions src/app/header/toolbar-button/toolbar-button.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ChangeDetectionStrategy, Component, Input } from '@angular/core'
import { ChangeDetectionStrategy, Component, input } from '@angular/core'
import { MaterialSymbolDirective } from '@/common/material-symbol.directive'

@Component({
Expand All @@ -10,6 +10,7 @@ import { MaterialSymbolDirective } from '@/common/material-symbol.directive'
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class ToolbarButtonComponent {
@Input({ required: true }) icon!: string
readonly icon = input.required<string>()
// https://github.com/angular/components/blob/18.0.5/src/material/button/button-base.ts#L257-L266
// https://github.com/angular/components/blob/18.0.5/src/material/button/button-base.ts#L257-L266
}
2 changes: 1 addition & 1 deletion src/app/resume-page/attribute/attribute.component.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<span [attr.aria-describedby]="_id" tabindex="0" appMaterialSymbol>
{{ symbol }}
{{ symbol() }}
</span>
<div [attr.id]="_id" role="tooltip"><ng-content></ng-content></div>
2 changes: 1 addition & 1 deletion src/app/resume-page/attribute/attribute.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('AttributeComponent', () => {

function makeSut() {
const [fixture, component] = componentTestSetup(AttributeComponent)
component.symbol = symbol
fixture.componentRef.setInput('symbol', symbol)
fixture.detectChanges()
return [fixture, component] as const
}
Expand Down
4 changes: 2 additions & 2 deletions src/app/resume-page/attribute/attribute.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, Input } from '@angular/core'
import { Component, input } from '@angular/core'
import { MaterialSymbolDirective } from '@/common/material-symbol.directive'

/**
Expand All @@ -16,7 +16,7 @@ let nextId = 0
imports: [MaterialSymbolDirective],
})
export class AttributeComponent {
@Input({ required: true }) symbol!: string
readonly symbol = input.required<string>()

protected _id = `${ATTRIBUTE_ID_PREFIX}${nextId++}`
}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<img [ngSrc]="src" width="64" height="64" [alt]="alt" />
<img [ngSrc]="src()" width="64" height="64" [alt]="alt()" />
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ describe('CardHeaderImageComponent', () => {
TestBed.configureTestingModule({})
fixture = TestBed.createComponent(CardHeaderImageComponent)
component = fixture.componentInstance

component.src = src
component.alt = alt
fixture.componentRef.setInput('src', src)
fixture.componentRef.setInput('alt', alt)
fixture.detectChanges()
})

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, Input } from '@angular/core'
import { Component, input } from '@angular/core'
import { NgOptimizedImage } from '@angular/common'

@Component({
Expand All @@ -8,6 +8,6 @@ import { NgOptimizedImage } from '@angular/common'
imports: [NgOptimizedImage],
})
export class CardHeaderImageComponent {
@Input({ required: true }) src!: string
@Input({ required: true }) alt!: string
readonly src = input.required<string>()
readonly alt = input.required<string>()
}
10 changes: 5 additions & 5 deletions src/app/resume-page/chip/chip.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ describe('ChipComponent', () => {

describe('when selected attribute is false', () => {
it('should not add the selected class', () => {
const [fixture, component] = componentTestSetup(ChipComponent)
component.selected = false
const [fixture] = componentTestSetup(ChipComponent)
fixture.componentRef.setInput('selected', false)
fixture.detectChanges()

expect(fixture.debugElement.classes['selected']).toBeFalsy()
Expand All @@ -26,8 +26,8 @@ describe('ChipComponent', () => {

describe('when selected attribute is true', () => {
it('should add the selected class', () => {
const [fixture, component] = componentTestSetup(ChipComponent)
component.selected = true
const [fixture] = componentTestSetup(ChipComponent)
fixture.componentRef.setInput('selected', true)
fixture.detectChanges()

expect(fixture.debugElement.classes['selected']).toBeTrue()
Expand Down Expand Up @@ -65,7 +65,7 @@ describe('ChipComponent', () => {

beforeEach(() => {
;[fixture, component] = componentTestSetup(ChipComponent)
component.selected = false
fixture.componentRef.setInput('selected', false)
subscription = component.selectedChange
.pipe(first())
.subscribe((selected) => (newSelectedValue = selected))
Expand Down
9 changes: 4 additions & 5 deletions src/app/resume-page/chip/chip.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
EventEmitter,
HostBinding,
HostListener,
Input,
input,
Output,
} from '@angular/core'

Expand All @@ -12,11 +12,10 @@ import {
template: '<ng-content></ng-content>',
styleUrls: ['./chip.component.scss'],
standalone: true,
host: { '[class.selected]': 'selected()' },
})
export class ChipComponent {
@HostBinding('class.selected')
@Input()
public selected?: boolean
public readonly selected = input<boolean>()

@Output()
public selectedChange = new EventEmitter<boolean>()
Expand Down Expand Up @@ -49,6 +48,6 @@ export class ChipComponent {
}

private emitToggledSelected() {
this.selectedChange.emit(!this.selected)
this.selectedChange.emit(!this.selected())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('ChippedContentComponent', () => {
chipElements.forEach((chipElement, index) => {
const content = CONTENTS[index]

expect(getComponentInstance(chipElement, ChipComponent).selected)
expect(getComponentInstance(chipElement, ChipComponent).selected())
.withContext(`chip ${index} is unselected`)
.toBeFalse()

Expand Down Expand Up @@ -102,7 +102,7 @@ describe('ChippedContentComponent', () => {

it('should mark the chip as selected', () => {
expect(
getComponentInstance(firstChipElement, ChipComponent).selected,
getComponentInstance(firstChipElement, ChipComponent).selected(),
).toBe(true)
})

Expand All @@ -128,7 +128,7 @@ describe('ChippedContentComponent', () => {

it('should mark the chip as unselected', () => {
expect(
getComponentInstance(firstChipElement, ChipComponent).selected,
getComponentInstance(firstChipElement, ChipComponent).selected(),
).toBeFalse()
})

Expand All @@ -154,11 +154,11 @@ describe('ChippedContentComponent', () => {

it('should mark the previous chip as unselected and just tapped chip as selected', () => {
expect(
getComponentInstance(firstChipElement, ChipComponent).selected,
getComponentInstance(firstChipElement, ChipComponent).selected(),
).toBeFalse()

expect(
getComponentInstance(secondChipElement, ChipComponent).selected,
getComponentInstance(secondChipElement, ChipComponent).selected(),
).toBeTrue()
})

Expand Down
6 changes: 3 additions & 3 deletions src/app/resume-page/date-range/date-range.component.html
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<span class="start">{{ range.start | date: 'MMM yyyy' }}</span>
<span class="start">{{ range().start | date: 'MMM yyyy' }}</span>
<span class="separator"></span>
<span class="end">
@if (range.end) {
{{ range.end | date: 'MMM yyyy' }}
@if (range().end) {
{{ range().end | date: 'MMM yyyy' }}
} @else {
Present
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('DateRangeComponent', () => {
})

function setRangeAndDetectChanges(range: DateRange) {
component.range = range
fixture.componentRef.setInput('range', range)
fixture.detectChanges()
}

Expand Down
4 changes: 2 additions & 2 deletions src/app/resume-page/date-range/date-range.component.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, Input } from '@angular/core'
import { Component, input } from '@angular/core'
import { DateRange } from './date-range'
import { DatePipe } from '@angular/common'

Expand All @@ -8,5 +8,5 @@ import { DatePipe } from '@angular/common'
imports: [DatePipe],
})
export class DateRangeComponent {
@Input({ required: true }) public range!: DateRange
public readonly range = input.required<DateRange>()
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import { educationItemToContents } from './education-item-to-contents'
],
})
export class EducationItemComponent {
// TODO: Skipped for migration because:
// Accessor inputs cannot be migrated as they are too complex.
@Input({ required: true }) public set item(item: EducationItem) {
this._item = item
if (item.institution.name.length > 15 && item.institution.shortName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import { experienceItemToContents } from './experience-item-to-contents'
],
})
export class ExperienceItemComponent {
// TODO: Skipped for migration because:
// Accessor inputs cannot be migrated as they are too complex.
@Input({ required: true }) public set item(item: ExperienceItem) {
this._item = item
this._contents = experienceItemToContents(item)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
<app-card>
<app-card-header>
<app-language-tag [tag]="item.tag"></app-language-tag>
<app-language-tag [tag]="item().tag"></app-language-tag>
<app-card-header-texts>
<app-card-header-title appTestId="name">
{{ item.name }}
{{ item().name }}
</app-card-header-title>
<app-card-header-subtitle appTestId="fluency">
{{ item.fluency }}
{{ item().fluency }}
</app-card-header-subtitle>
</app-card-header-texts>
</app-card-header>
@if (item.comment) {
@if (item().comment) {
<div appTestId="comment">
<p>{{ item.comment }}</p>
<p>{{ item().comment }}</p>
</div>
}
</app-card>
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,6 @@ function setLanguageItem(
fixture: ComponentFixture<LanguageItemComponent>,
overrides?: Partial<LanguageItem>,
) {
fixture.componentInstance.item = makeLanguageItem(overrides)
fixture.componentRef.setInput('item', makeLanguageItem(overrides))
fixture.detectChanges()
}
Loading

0 comments on commit 0090c07

Please sign in to comment.