lisaoude / gifinder Goto Github PK

If you're new to js, some function variables and parameters may be unfamiliar to you.

For example

function (e)

means:

function (event)

fetch(url).then(res)

means:

fetch(url).then(response)

Sometimes its easier to write things out in the beginning so that you become familiar with these names and afterwards use the shorthand

Feedback

• There is no Actor Diagram and Interaction Diagram available yet so I have no idea if it matches your code. 😞
• Based on your explanation in your README, it is clear what I could expect with your live demo. ✅
• I myself would have made the application a bit more exciting. But maybe you can still do this (even if it's small things) and maybe still focus on the styling? I know that you are good with styling and CSS ! 👍 👍

To Solve This Issue:

• Make your site a bit more exciting (styling / general concept).

It works!
The concept is pretty simple. You could definitely spice up your styling with some css!

Feedback

• No idea if your code is structured according to your diagrams as they are not yet in your readme. 😞
• It is better to put your script tag in your body at the bottom. Because you give the reason that you have to wait for the html elements. But you load your html elements with JavaScript. 😄
• In your main.js you have a comment: 'this function shows the trending gifs'. But I think the 'start' function does not specifically show the trending gifs but rather invokes your router. 😉
• You now have an index.js file within your modules views folder that you don't use. I think you can remove these. 😄
• Do you only have 1 component called gifs.js?
• Are your views, config and components just modules?
• I wonder if you understand all your code correctly yourself... 🤯

To Solve This Issue:

• Add script tag to bottom of body tag in HTML.
• Add comments that make sense for your code.
• Remove your index.js file within your modules views folder.
• Try to take a look at what modules are, what components are and how your code actually works.

Algemeen/repo

• Read me goede manier om alles bij te houden! handig dat je een to do list voor jezelf hebt gemaakt.
• Api/concept is beschreven en de link naar de live demo staat er ook! :)
• Je mist alleen nog een description bij de repo waar ook je link staat zeg maar.
• Voor volgende keer: include actor en interaction diagram :)
• Live demo werkt, ik verwachtte precies wat ik dacht.

Code

• const init (in main.js) ik snap zelf de benaming init niet 😬, misschien dat je dat search noemt? dan wordt het iets duidelijker.
• Verder ziet je code er netjes uit, ik snap wat er gebeurt! 😊
• De actors zijn helder, main.js (zoeken naar specifieke gif), fetchData (haalt de data uit Api), printGifs (rendert data in html), die nu worden geïmporteerd naar main.
• Als tip misschien bovenin elke script bestand in module nog een comment aangeven wat er binnen dat script gebeurd. Handig voor jezelf voor later 😄
• Code is consistent overal zelfde methode!

Linking to external detailpages is cheating ;-)

https://github.com/lisaoude/web-app-from-scratch-2021/blob/221021ee2f54d108538b6d42a765bc8d44aa909a/js/modules/components/gifs.js#L14

the nodelist has its own forEach() method

https://github.com/lisaoude/web-app-from-scratch-2021/blob/221021ee2f54d108538b6d42a765bc8d44aa909a/index.html#L23-L26

You gets submit functionality out of the box (which also works with the keaboard)

https://github.com/lisaoude/web-app-from-scratch-2021/blob/221021ee2f54d108538b6d42a765bc8d44aa909a/js/modules/data/fetch.js#L5

It might be better to use the more versatile devtools debugger anyway?
https://javascript.info/debugging-chrome

• Link to demo
• The description is a bit short on the concept
• The used api is not described in the readme
• Place an image of the actor and interaction diagrams in the readme

So usually what shouldn't be in a repo is your weekly log and the process of making your app. The readme must contain the following:

• Description of Concept
• If there is an API, description of API
• Actor and interaction diagram (this assignment)
• If you need to install this project, an installation guide i.e. npm install etc.
• Acknowledgments (shoutouts to the people who helped you make this)

The rest of your documentation should be placed inside a wiki

Feedback

• There is a link at the top right to the live demo. ✅
• You have described your concept (being able to search for gifs). ✅ Maybe if you have the time and desire you can expand this concept a bit so that it becomes a bit more exciting (a button to save gifs for example). 😄
• The API is described in the README file, but maybe you can tell a bit more about it. Consider, for example: How did you use the API to load the GIFs, how did you use the API so that the user can search for the GIFs, etc.
• The actor & interaction diagram is not yet available. 😞

General tips:

• Try to say in your README file how people can download / use your repo. Maybe you have used a package that makes downloading your repo only not work on someone else's computer. 😄
• Maybe you can show in your README file which sources you have used. 😃
• When making commits, try to describe briefly and clearly what you have changed for yourself. I also see commits with 'IT WORKS' 'Changed many many many things'. This makes it difficult for you to review your work later. 😸

To Solve This Issue:

• Describe a little more how you use your API.
• Add the Actor Diagram to your README file.
• Add the Interaction Diagram to your README file.
• Add resources to your README.
• Describe how someone else can download your repo.

Every code has their own conventions, that of classes and id's in html is that you write you them like this:

<section class="gif-type">

https://github.com/lisaoude/web-app-from-scratch-2021/blob/221021ee2f54d108538b6d42a765bc8d44aa909a/js/main.js#L11

Try to be consitent in chosing for const or let

In your code you have:

const gifContainer = document.querySelector('.gifContainer')
const input = document.querySelector('.gifType')
const form = document.querySelector('.gifForm')

and

    const
        endpoint = 'https://api.giphy.com/v1/gifs/search?q=',
        key = 'JzbS3qH49ykiVKnHZx0z5M0gjNPVMbh3',
        limit = 20,
        url = `${endpoint}${inputValue}&api_key=${key}&limit=${limit}`

Try to keep your code consistent by using one of the two styles, so with writing all the consts/lets/vars or one var and then ,

• Complete actor diagram is missing
• Interaction diagram is missing

Code quality

• For someone who is new to coding, your code looks very clean, I'm impressed🤓👍
• Code is written the same way in different places
• Code follows naming conventions

https://github.com/lisaoude/web-app-from-scratch-2021/blob/221021ee2f54d108538b6d42a765bc8d44aa909a/index.html#L30

Load script from head instead

https://github.com/lisaoude/web-app-from-scratch-2021/blob/221021ee2f54d108538b6d42a765bc8d44aa909a/js/modules/components/gifs.js#L3

Algemeen

Nog geen app beschrijving in de readme, overall structuur is netjes en de github pages link werkt. 👍🏻
Je zou nog even kunnen kijken naar het geven van een beschrijving van een commit. Op die manier kan je makkelijk zien wat er in die commit allemaal veranderd is, als je alleen werkt niet super relevant maar heel handig in een team

HTML

HTML is semantisch correct, je zou eventueel je gifContainer in een section veranderen.

CSS

Er is nog geen duidelijk ontwerp maar de CSS die er is ziet er netjes uit

JS

Opbouw is logisch, er staan geen comments in wat kan helpen met het begrijpen van de code voor jou en andere. Verder erg netjes

🤓

If you're new to js, some function variables and parameters may be unfamiliar to you.

For example

function (e)

means:

function (event)

fetch(url).then(res)

means:

fetch(url).then(response)

Goede read me, alles staat er in. Als je in elements.js een functie maakt om je const's heen, dan hoef je alleen die functie te exporteren en niet al die variabelen. Zo heb ik t gedaan iig. Je fetchDtata.js ziet er goed uit. Goed dat je je link zo hebt opgesplitst in key, limit etc. Op je live demo werkt je zoek button maar een keer en je kan niet terug, dus miss een terug knop of een manier vinden om die zoek knop dan ook weer te laten werken. En als je nu naar je website gaat staat er geen content totdat je zoekt. Miss een meest populaire gifs lijst als 'home page'.

🤓🤓🤓🤓🤓🤓
Code is pretty clean, there are some small inconsistencies but overall really nice
🤓🤓🤓🤓🤓🤓

🤓🤓🤓🤓🤓🤓
HTML is semantic!
🤓🤓🤓🤓🤓🤓

🤓🤓🤓🤓🤓🤓
You rewrote your readme!
🤓🤓🤓🤓🤓🤓