Skip to content

MovieApp Project by Marina & Sara#49

Open
SaraEnderborg wants to merge 7 commits intoTechnigo:mainfrom
SaraEnderborg:main
Open

MovieApp Project by Marina & Sara#49
SaraEnderborg wants to merge 7 commits intoTechnigo:mainfrom
SaraEnderborg:main

Conversation

@SaraEnderborg
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.

Really good work with the theme file overall! I like that you included the genres on the movie details page as well.
The theme is not really meant to keep info like the API URLs, but I like how you thought about it! You could have made a separate file or hook for the API URLs and API key, like a useApi hook.
There is a bug on the movie details page, if you reload the page once on that page, it results in a not found page. It probably has something to do with how things are set up in Netlify, worth looking into.
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().
Overall great 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.

2 participants