rowinruizendaal / web-app-from-scratch-2021

Web App From Scratch @cmda-minor-web 2020 - 2021

Hi there 👋

Tech and Tools

Current favourites

Other tools I use

"The best error message is the one that never shows up."

web-app-from-scratch-2021's Issues

Readme.md Features

Issues

Close issues als je ze gesolved heb houd het voor jezelf ook overzichtelijk wat je nog gaat/moet doen.

Peer review week 2

Repository

Scan your buddy's repository
Is there a link to the live demo at the top?

• Die is er wel, maar hij werkt niet correct :)

Is the concept described well in the readme?

• Het concept wordt niet heel uitgebreid uitgelegd, maar wordt wel met gebruik van screenshots van de app duidelijker gemaakt.

Is the API described in the readme?

• Ja, de API wordt uitgelegd, en ook wordt de code gedeeld over het fetchen vanuit de API. Bovendien staat er ook een link naar de API-site.

Are the Actor and Interaction diagrams included?

• Ja!

Code

Actor Diagram
Using the Actor diagram, figure out the structure of the code
Do the actors make sense to you?

• De actors wel, maar er zit geen flow tussen de actors. Hierdoor kan ik dus niet duidelijk zien welke actors er met elkaar "communiceren".

Do you see any function you wouldn't expect for a certain Actor?

• Nee, het spreekt allemaal voor zich :)

Interaction Diagram
Using the interaction diagram, figure out how the app functions
Does the flow make sense to you?

• Ja, duidelijk en overzichtelijk uitgelegd.

Does anything work differently than you would code it?

• Ik zou misschien de flows tussen de elementen aanpassen, zodat er duidelijk te zien is welke elementen er worden aangeroepen in het interaction diagram.

Click through the live demo
Does it work the way you would expect it to?

• Ja!! Ziet er super vet uit, en werkt ook helemaal prima. Alleen is soms de afbeelding van een nummer/album niet beschikbaar, maar dit komt door de API.

What features or improvements would you like to see?

• Misschien een filter mogelijkheid, of zoekbalk? Hierdoor kun je specifiek zoeken naar een bepaald nummer/genre.

BROWSE the code
Has it already been structured according to the diagrams?

• Ja.

Check the code for consistency, is code being written the same way in different places?

• Zeker, de code is consistent geschreven, met een duidelijke structuur, wat zorgt voor een overzichtelijk script.

Does the code follow naming conventions? Do variable and function names make sense to you?

• Er worden goeie variabelnamen gebruikt, erg compact en duidelijk. Way to goooo.

Yoo! Je app ziet er goed uit man. Super nice om te zien, en om te gebruiken.
Blijf zo doorgaan!!

--Johan

Clean code

web-app-from-scratch-2021/js/modules/carousel/fillCarousel.js

Line 36 in 07bd010

// const results = document.querySelector('.results h3').textContent = `${data.length} results for ${artist}`

kan dit weg?

Feedback

App ziet er goed uit, erg strak.
Code is ook netjes, en werkend. Niets aan toe te voegen.

#Repository
Er is geen repo-description, en de repo bevat ook geen readme. De API is dus ook niet uitgelegd in de readme.

#Code
De code is strak, consistent, en op volgorde.
Duidelijke id/class variabelen. Code flow is goed te lezen.

Carousel mis ik wel, maar dat komt door een mislukte push, maar de code ziet er goed uit

Geen flow tussen actors

Ik mis de flow in tussen de actoren in het actor diagram die aantonen hoe deze samen zouden werken

Keuzes in diagram horen volgensmij met een ruit vorm

Feedback

web-app-from-scratch-2021/js/app.js

Line 24 in bb6122c

const results = document.querySelector('.results h3').textContent = `${data.length} results for ${artist}`

Dit klopt niet helemaal een mooiere oplossing zou zijn:

const results = document.querySelector('.results h3');
results .textContent = `${data.length} results for ${artist}`;

Voordeel hieraan is dat je results nu weer opnieuw zou kunnen wijzigen door results.textContent = "jouw waarde"
In jouw geval zou je een nieuwe variabele moeten aanmaken en eerst weer een querySelector moeten doen.

Readme.md API

Mis nog wat info over je API

• aantal requests?

Readme.md Features

wat kan ik met de app doen?

wat wil je nog maken?

Dit zijn leuke dingen om in je readme op te nemen.

Mapping instead of for loops

Peer review web app from scratch 20/21 #week 1

De code bevat geen syntaxfouten en is netjes opgemaakt

De structuur is duidelijk en logisch opgezet. Misschien wel nu al met modules werken though? Of was je dat vanaf week 2 van plan?

Bij het maken van functies en variabelen wil je de intentie (intent) waarvoor je de variabel/functie gaat gebruiken weergeven en niet zo zeer wat er precies gebeurd in de variabel/functie. Is het niet verstandiger om voor functienamen de intentie te beschrijven in plaats van de naam van de entiteit?

Verder wil je ook met abstractielagen werken in methods. Elke method bestaat uit maar 1 abstractielaag. Ook dat doe je goed.

JSON data kan met een asynchrone request worden opgehaald uit een API

Het is je gelukt om data asynchroon met async/await op te halen uit de API.

JSON data kan, al dan niet met een micro library, client side in de HTML worden gerenderd.

Het is je gelukt om gebruik te maken van het renderen van client side code in de HTML zoals we zien bij de arrow function carousel.

Je carousel ziet er heel mooi uit. Het is ook mobile responsive. Heel goed dat je daar nu al over nadenkt!

Verder zie ik niet dat je jezelf aan het herhalen bent in je code (DRY principe) en je maakt gebruik van scopes.

Over de Readme

De readme moet nog geupdate worden zie ik. Maar dat wist je zelf ook al :)!

Repository peer feedback week 2

Repository
Hey Rowin, bij deze mijn peer review op jouw repository voor de WAFS API opdracht, week 2.

Zie de vinkjes (✔️) en kruisjes (❌) niet als fout of goed, maar een vinkje is voor als je heta af hebt, en een kruisje voor als er nog iets mist :).

✔️ Is there a link to the live demo at the top?
Jazeker! Alleen nog een tip voor je 'description': ik zou hier nog even in het kort uitleggen wat/ waar voor het project is. En misschien wat tags toevoegen is altijd leuk :).

✔️ Is the concept described well in the readme?
Je hebt bij je description geschreven dat je doormiddel van de API, de huidige nummers van de artist kan ophalen, wat goed is! Ik zou alleen nog even beschrijven wat je app allemaal kan.

✔️ Is the API described in the readme?
Ja! Goede beschrijven wat de API is en hoe je data fetched.

✔️ Are the Actor and Interaction diagrams included?
Ja!

Keep coding~~

Code peer feedback week 2

Code
Hey Rowin, bij deze mijn peer review op jouw code voor de WAFS API opdracht, week 2.

Zie de vinkjes (✔️) en kruisjes (❌) niet als fout of goed, maar een vinkje is voor als je heta af hebt, en een kruisje voor als er nog iets mist :).

Using the Actor diagram, figure out the structure of the code

✔️ Do the actors make sense to you?
Ja zeker, alle belangrijke actors worden mee genomen (voor zover ik weet).

✔️ Do you see any function you wouldn't expect for a certain Actor?
Is zie geen rare functies!

Using the interaction diagram, figure out how the app functions

✔️❌ Does the flow make sense to you?
Misschien kan je de 'On page load' en 'On first time vist' iets breeder uit elkaar zetten, ik weet nu niet zo goed waar ik moet beginnen. Voor de rest is de flow erg goed te volgen, ook goed dat je opties van wat er gebeurd als iets wel en niet lukt!

Click through the live demo

✔️❌ Does it work the way you would expect it to? / What features or improvements would you like to see?

• Ik vind het duidelijk dat er onder aan de pagina de artiesten staan, maar misschien kan je hier nog een kopje zetten met 'Artist'. Dit maakt het makkelijk voor de gebruiker om te zien wat het nou eigenlijk is.
• Zodra ik op een artist heb gedrukt, zie ik een duidelijke carousell, ik snap dat ik er doorheen kan slepen en de nummers kan afspelen! Alleen zie ik geen knopje om terug te gaan, en terug gaan met mn muis werkt ook niet.. alleen refresh natuurlijk. Misschien kan je hier nog naar kijken.

Ideetjes:
Geen idee of het kan, maar het lijkt mij wel vet om de meest geluisterde nummers op de hoofdpagina te zien.

Browse the code

✔️❌ Has it already been structured according to the diagrams?
De namen komen niet perse overeen, maar je hebt wel alle elementen.

✔️ Think of structural improvements
Goed gedaan! Hier weet ik geen feedback voor.

✔️Check the code against our best practices
Je gebruikt innerHTML, wat goed is, maar bedenk goed of dit de goede oplossing is voor jezelf! (ik vind van wel, maar let hier wel op :))

✔️Check the code for consistency, is code being written the same way in different places?
Jep it does! Goed op gelet.

✔️ Does the code follow naming conventions? Do variable and function names make sense to you?
De enige naam die mij verward, is secondcontainer2 haha.. Is dit de 2de container van de 2de container? Voor de rest goed gedaan.

Keep coding~~