Skip to content

Conversation

@jonathanrasquin
Copy link

No description provided.

@for(sock of socks$ | async; track sock.id){
<div class="col-sm-6 col-md-4 col-lg-3">
<div class="box">
<a href="/socks/{{ sock.id }}">
Copy link
Member

Choose a reason for hiding this comment

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

Hier wel opletten!!

Als je een <a href=""> doet, dan gaat de browser daar naartoe navigeren, je hebt daarmee een page reload!!
Ofte: je verliest de app state + een re-render van de volledige pagina

Je moet met routerLink werken zodat je binnen de SPA blijft!

</div>
</div>
<div>
<button (click)="onPageChange(currentPageSubject.value - 1)" [disabled]="currentPageSubject.value === 1">Previous</button>
Copy link
Member

Choose a reason for hiding this comment

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

Nice use van [disabled] !!

public colorFilterSubject = new BehaviorSubject<string>('');
public sortBySubject = new BehaviorSubject<string>('name');
public currentPageSubject = new BehaviorSubject<number>(1);
public pageSize = 10;
Copy link
Member

Choose a reason for hiding this comment

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

Public is default, je hoeft dit dus niet speciaal te vermelden
Op sommige projecten doen ze dat wel --> volg dan de team/project standaarden

}

onFilterColor(event: Event) {
const color = (event.target as HTMLInputElement).value;
Copy link
Member

@Laoujin Laoujin May 31, 2024

Choose a reason for hiding this comment

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

Dit is een beetje een probleem in Angular, je zou hierrond kunnen werken met wat TypeScript magic:

public onChange(event: Event & { target: HTMLInputElement }): void { }

Maar of dat dan beter is....

const startIndex = (currentPage - 1) * this.pageSize;
return filtered.slice(startIndex, startIndex + this.pageSize);
})
);
Copy link
Member

Choose a reason for hiding this comment

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

Components, components, components!!

Je zit in een vrij groot ShopComponent waarin nu vanalle logica komt te staan (paginatie, filtering, sorting, ...)
En later ook nog 101 andere dingen

Zulke components zijn moeilijk testbaar (als er ooit testen zouden geschreven worden ;) )
En ook moeilijk begrijpbaar, er is wat vanalles aan de gang. Na een tijdje heb je geen idee meer voor welke functionaliteit welke code precies verantwoordelijk is

  • in sommige functies (vb ngOnInit) zit er voor elk van die dingen wat code...

SRP --> Maak meer en kleinere componenten die elk een heel duidelijk afgelijnd doel/purpose hebben

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.

2 participants