Skip to content

Move project hand in from Asako and Frida #48

Open
fridascript wants to merge 14 commits intoTechnigo:mainfrom
fridascript:main
Open

Move project hand in from Asako and Frida #48
fridascript wants to merge 14 commits intoTechnigo:mainfrom
fridascript:main

Conversation

@fridascript
Copy link
Copy Markdown

Netlify link here.

Copy link
Copy Markdown

@Npahlfer Npahlfer left a comment

Choose a reason for hiding this comment

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

Looking at the commits, try to distribute the workload a bit more evenly in the team. If you've pair coded a lot and then committed more from one person then that's fine and ignore this comment :)
Make sure to keep reused URLs like the API URL in one place, as a constant, so it's easy to change and kept central when used in more than one file. The same goes for the API key, that could even be put in a .env file :)
When fetching data and handling loading states, try to make use of finally. That way you don't have to set the loading state to false both in your last .then() and in the .catch().
Make use of the styled component theme to not repeat yourselves, for example with media queries and colors.
Would have been nice to see some more stretch goals in order to set a higher grade, but overall good job!

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