Skip to content

Movie Project by Nicolina and Julia#50

Open
julialindstrand wants to merge 12 commits intoTechnigo:mainfrom
julialindstrand:main
Open

Movie Project by Nicolina and Julia#50
julialindstrand wants to merge 12 commits intoTechnigo:mainfrom
julialindstrand:main

Conversation

@julialindstrand
Copy link
Copy Markdown

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.

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 doing fetch requests, try to wrap it in a try/catch so that errors, if any, can be caught and displayed to the user in a good way. When fetching data and handling application state, you can also make use of finally. That way you don't have to set the loading state to false both after the state update and in the .catch().
When defining variables, like you do in the Movie component with OneMovieData, don't use capitalized words for variables that aren't classes or components. If a "normal" variable like this is capitalized, the reader might think it's something else than just a holder of data.
Make use of the styled component theme to not repeat yourselves, for example with media queries and colors.
It would have been nice to see some of the stretch goals in there, but otherwise good work!

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