Skip to content

Conversation

@YannickManfroy
Copy link

No description provided.

</section>


<app-sock-reviews [sockId]="this.sockId"></app-sock-reviews>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi :: in de template kan je de this. laten vallen (weeral 5 characters code minder ;) )

templateUrl: './sock-reviews.component.html'
})
export class SockReviewsComponent {
@Input() sockId!: number | undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Het ! wil zeggen dat de value altijd gezet is (voor TypeScript, niet voor Angular)
Voor Angular is dat met @Input({required: true}) sockId: number

Aangezien je hier undefined ook accepteert

@Input() sockId: number;

// HACK: This is not the way to get the sockId!!
const sockId = +window.location.pathname.split('/')[2];
this.sock$ = this.socksService.getById(sockId);
this.sockId = +window.location.pathname.split('/')[2];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probeer fields altijd te vermijden.

Een van de oefeningen is "Maak een next button om naar de volgende sock te gaan"
Hiervoor moet je dan een ngOnChanges gaan doen (en later nog andere dingen)

Je kan vb met een getter werken:

get sockId(): number {
  return +window.location.pathname.split('/')[2];
}

(maar natuurlijk is die window.location NOT the Angular way to go! (nog een andere oefn;)

@@ -1,3 +1,12 @@
@if ((reviews$ | async)?.length === 0){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ATTENTION:
Heb je de network tab in de gaten gehouden?
Ben je hier 2x de reviews aan het ophalen?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants