Lesson 14

[Sprint Three] - Refactoring

Clearing our our refactoring tasks

PRO

Lesson Outline

Sprint Three - Refactoring

It's time to begin out next sprint. To recap for those of you not following along with the project management module, these are the tasks that we are including in this sprint:

  • #78 refactor: dumb components should define their own loading skeleton
  • #77 refactor: client detail page data into dumb component
  • #83 chore: set up basic PWA configuration and styling
  • #67 feat: ability to log out
  • #68 feat: ability to delete a client
  • #17 feat: mechanism for clients to supply feedback anonymously
  • #18 feat: view feedback that has been submitted anonymously

Refactor: Dumb components

Let's start with one of our refactoring tasks:

refactor: client detail page data into dumb component


Project management

Remember to move the card for this task to the Test column, and create a new task branch to work on.


Our ClientDetailPage has gotten a little out of hand:

<ion-header>
  <ion-toolbar>
    <ion-title>
      <ng-container *ngIf="client$ | async as client">
        {{client.name.first}} {{client.name.last}}
      </ng-container>
    </ion-title>
    <ion-buttons slot="start">
      <ion-back-button
        defaultHref="/clients"
        data-test="client-detail-back-button"
      ></ion-back-button>
    </ion-buttons>
    <ion-buttons slot="end">
      <ion-button
        *ngIf="client$ | async as client"
        routerLink="/clients/{{client.id}}/edit"
        routerDirection="forward"
        data-test="edit-button"
      >
        <ion-icon slot="icon-only" name="create-outline"></ion-icon>
      </ion-button>
    </ion-buttons>
  </ion-toolbar>
</ion-header>

<ion-content class="ion-padding">
  <ng-container *ngIf="client$ | async as client; else loading">
    <ion-card>
      <ion-card-header>
        <ion-card-title data-test="client-name-display">
          {{client.name.first}} {{client.name.last}}
        </ion-card-title>
      </ion-card-header>

      <ion-card-content>
        <ion-item lines="none" data-test="client-email-display">
          <ion-icon slot="start" name="mail-outline"></ion-icon>
          <ion-label> {{client.email}} </ion-label>
        </ion-item>

        <ion-item lines="none" data-test="client-phone-display">
          <ion-icon slot="start" name="call-outline"></ion-icon>
          <ion-label> {{client.phone}} </ion-label>
        </ion-item>
      </ion-card-content>
    </ion-card>

    <ion-card>
      <ion-card-content>
        <p data-test="client-notes-display">{{client.notes}}</p>
      </ion-card-content>
    </ion-card>
  </ng-container>
  <ng-template #loading>
    <ion-card data-test="loading">
      <ion-card-header>
        <ion-card-title>
          <ion-skeleton-text [animated]="true"></ion-skeleton-text>
        </ion-card-title>
      </ion-card-header>

      <ion-card-content>
        <ion-item lines="none">
          <ion-icon slot="start" name="mail-outline"></ion-icon>
          <ion-skeleton-text [animated]="true"></ion-skeleton-text>
        </ion-item>

        <ion-item lines="none">
          <ion-icon slot="start" name="call-outline"></ion-icon>
          <ion-skeleton-text [animated]="true"></ion-skeleton-text>
        </ion-item>
      </ion-card-content>
    </ion-card>
  </ng-template>
</ion-content>

Remember that our goal is generally to have a smart parent component, and dumb presentational components responsible for presenting things to the user. Our smart component is currently doing everything on this page.

Let's remove the cards that display the user's information into a dumb component:

ionic g component clients/ui/ClientDetailCard --create-module --export

Before we move our code over, let's write unit tests for how it will deal with its client input:

import { ChangeDetectionStrategy } from '@angular/core';
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { IonicModule } from '@ionic/angular';
import { Client } from '../../data-access/clients.store';

import { ClientDetailCardComponent } from './client-detail-card.component';

describe('ClientDetailCardComponent', () => {
  let component: ClientDetailCardComponent;
  let fixture: ComponentFixture<ClientDetailCardComponent>;
  let testClient: Client;

  beforeEach(waitForAsync(() => {
    TestBed.configureTestingModule({
      declarations: [ClientDetailCardComponent],
      imports: [IonicModule.forRoot()],
    })
      .overrideComponent(ClientDetailCardComponent, {
        set: { changeDetection: ChangeDetectionStrategy.Default },
      })
      .compileComponents();

    fixture = TestBed.createComponent(ClientDetailCardComponent);
    component = fixture.componentInstance;

    testClient = {
      name: {
        first: 'Josh',
        last: 'Morony',
      },
      email: '[email protected]',
      phone: '555',
      notes: '',
      survey: [],
      appointments: [],
    };

    component.client = testClient;

    fixture.detectChanges();
  }));

  it('should create', () => {
    expect(component).toBeTruthy();
  });

  describe('@Input() client', () => {
    it('displays the clients name', () => {
      const nameDisplay = fixture.debugElement.query(
        By.css('[data-test="client-name-display"]')
      );

      expect(nameDisplay.nativeElement.textContent).toContain(
        `${testClient.name.first} ${testClient.name.last}`
      );
    });

    it('displays the clients email', () => {
      const emailDisplay = fixture.debugElement.query(
        By.css('[data-test="client-email-display"]')
      );

      expect(emailDisplay.nativeElement.textContent).toContain(
        `${testClient.email}`
      );
    });

    it('displays the clients phone', () => {
      const phoneDisplay = fixture.debugElement.query(
        By.css('[data-test="client-phone-display"]')
      );

      expect(phoneDisplay.nativeElement.textContent).toContain(
        `${testClient.phone}`
      );
    });

    it('displays the clients notes', () => {
      const notesDisplay = fixture.debugElement.query(
        By.css('[data-test="client-notes-display"]')
      );

      expect(notesDisplay.nativeElement.textContent).toContain(
        `${testClient.notes}`
      );
    });
  });
});

We will need to define the client input before running the tests:

import { ChangeDetectionStrategy, Component, Input } from '@angular/core';
import { Client } from '../../data-access/clients.store';

@Component({
  selector: 'app-client-detail-card',
  templateUrl: './client-detail-card.component.html',
  styleUrls: ['./client-detail-card.component.scss'],
  changeDetection: ChangeDetectionStrategy.OnPush,
})
export class ClientDetailCardComponent {
  @Input() client: Client;

  constructor() {}
}

If we run the tests, this should give us a bunch of failures like this:

  ● ClientDetailCardComponent - @Input() client - displays the clients notes

    TypeError: Cannot read properties of null (reading 'nativeElement')

Add the following to client-detail-card.component.html:

<ion-card>
  <ion-card-header>
    <ion-card-title data-test="client-name-display">
      {{client.name.first}} {{client.name.last}}
    </ion-card-title>
  </ion-card-header>

  <ion-card-content>
    <ion-item lines="none" data-test="client-email-display">
      <ion-icon slot="start" name="mail-outline"></ion-icon>
      <ion-label> {{client.email}} </ion-label>
    </ion-item>

    <ion-item lines="none" data-test="client-phone-display">
      <ion-icon slot="start" name="call-outline"></ion-icon>
      <ion-label> {{client.phone}} </ion-label>
    </ion-item>
  </ion-card-content>
</ion-card>

<ion-card>
  <ion-card-content>
    <p data-test="client-notes-display">{{client.notes}}</p>
  </ion-card-content>
</ion-card>

and now let's check the tests:

 FAIL  src/app/clients/feature/client-detail/client-detail.page.spec.ts
  ● ClientDetailPage - loading state - should display data if clients$ has emitted a value

    expect(received).toBeTruthy()

They do, but we've broken the ClientDetailPage now. Let's fix that by adding our dumb component to the template. First, we will need to add the module for the dumb component to ClientDetailPage:

@NgModule({
  imports: [
    CommonModule,
    FormsModule,
    IonicModule,
    ClientDetailPageRoutingModule,
    ClientDetailCardComponentModule,
  ],
  declarations: [ClientDetailPage],
})
export class ClientDetailPageModule {}

Then we can add it to the template:

<ion-content class="ion-padding">
  <app-client-detail-card
    *ngIf="client$ | async as client; else loading"
    [client]="client"
  >
  </app-client-detail-card>
  <ng-template #loading>
    <ion-card data-test="loading">
      <ion-card-header>
        <ion-card-title>
          <ion-skeleton-text [animated]="true"></ion-skeleton-text>
        </ion-card-title>
      </ion-card-header>

      <ion-card-content>
        <ion-item lines="none">
          <ion-icon slot="start" name="mail-outline"></ion-icon>
          <ion-skeleton-text [animated]="true"></ion-skeleton-text>
        </ion-item>

        <ion-item lines="none">
          <ion-icon slot="start" name="call-outline"></ion-icon>
          <ion-skeleton-text [animated]="true"></ion-skeleton-text>
        </ion-item>
      </ion-card-content>
    </ion-card>
  </ng-template>
</ion-content>

Now there are a couple more problems we need to deal with - we are getting some errors in our tests. We also still have a lot in the template above, but we are going to be addressing refactoring the loading skeleton component in the next issue.

Our first problem is that our test does not know about the app-client-detail-card so we are going to need to provide a mock for that. Just like last time, let's define a mock for this component in the spec file for the component itself.

Define the following mock in client-detail-card.component.spec.ts:

PRO

Thanks for checking out the preview of this lesson!

You do not have the appropriate membership to view the full lesson. If you would like full access to this module you can view membership options (or log in if you are already have an appropriate membership).