Review #5
Review #5
Conversation
AllanSaleh
left a comment
AllanSaleh
left a comment
There was a problem hiding this comment.
Great work team! 👏
Your code is looking great! Really like how you guys fragmented your code into components that make sense and separated the concerns. 👌
However, when I change the Genre, the grid of movies is not updated with the respected Genre. Other than that your project is superb!
I am very happy with the progress you've done and I'm proud of you! 🙌
Keep it up, team! 🔥
abdulbasit-hussein-yazen/src/App.js
Outdated
| const [movies, setMovies] = useState([]); | ||
| const [trending, setTrending] = useState([]) | ||
| window.addEventListener('DOMContentLoaded',() => { | ||
| fetch("https://api.themoviedb.org/3/trending/movie/day?api_key=754ad3989128c7d8cfcc82e6591e7f2e") |
There was a problem hiding this comment.
I know the construct URL is was only optimized for the search box and was not required to be used here but just for info, best practice would be to optimize the construct URL function to construct the URL and making sure the function would construct the URL whether it has a query or not.
Even though, you might think that it's overkill, it really is not, as soon as you get the function to construct URLs correctly, you wouldn't need to worry about the structure of URLs for other areas in your web app.
| // const handleChange = (e) => { | ||
| // if (e.target.value) { | ||
| // setSpinner("d-block"); | ||
| // fetch(constructUrl("search/movie", e.target.value)) | ||
| // .then((response) => response.json()) | ||
| // .then((response) => props.get(response.results)); | ||
| // } else { | ||
| // setSpinner("d-none"); | ||
| // } | ||
| // }; |
There was a problem hiding this comment.
Please remove any commented out code before submitting a pull request for review.
| fetch(constructUrl("search/movie", searchText)) | ||
| .then((response) => response.json()) | ||
| .then((data) => { | ||
| props.setMovies(data.results); |
There was a problem hiding this comment.
Awesome job on lifting the state 2 stages. This is very well done. Hats off to you! 🎩
| <Nav.Link href="#home">Home</Nav.Link> | ||
| <GenresDropdown setMovies={props.setMovies}/> | ||
| </Nav> | ||
| <SearchBox setMovies={props.setMovies} /> |
There was a problem hiding this comment.
Great work extracting the searchbox into a separate component and using it in the navbar. Also passing the setMovies function to it and doing the logic in the searchbox component! 👏
2 participants
This is the link to the version 3 : https://husseintalal2.github.io/iraq-bc-movies-project-students/