Review latest version ! #2
Conversation
MustafaMerie
left a comment
MustafaMerie
left a comment
There was a problem hiding this comment.
very clean and good looking code. Good job team!
just a note: I noticed the genre feature is not working, so for the future try not to include features that you are still working on it for reviewing.
Evan-Sara-Jalal/movie-app/src/App.js
Outdated
| function handleSubmit(e) { | ||
| //e.preventDefault(); | ||
| // console.log(e.target[0].value); | ||
| // console.log("it is working "); |
There was a problem hiding this comment.
usually, comments should be removed before pushing the code for the review
| @@ -0,0 +1,10 @@ | |||
| import React, { Component } from "react"; | |||
There was a problem hiding this comment.
do you think we need to import Component here?
| overview={movie.overview} | ||
| popularity={movie.popularity} | ||
| release_date={movie.release_date} | ||
| key={index} |
There was a problem hiding this comment.
for a short cut, you can pass the movie object to Movie component and in that component put what you want to you form it
ex:
{props.movies.map((movie) => (<Movie movie={movie} />))}
in Movie component :
<h1>{movie.original_title}</h1>
<p>{movie.release_date}</p>
and so on ...
MustafaMerie
left a comment
MustafaMerie
left a comment
There was a problem hiding this comment.
you did a great job team! loved your work.
left some comments to make your project even better!
| import React, { Component } from "react"; | ||
| import MovieGrid from "./movieGrid"; | ||
| import { useState } from "react"; | ||
| import MoviePage from "./moviePage"; |
There was a problem hiding this comment.
Delete the not used imported files or hooks or anything. fix here and all.
| <h2>Actors</h2> | ||
| <hr /> | ||
| {movie.id && | ||
| cast.map((actor) => { |
There was a problem hiding this comment.
I will prefer it if you used slice() with map() here, and show more info about the actor.
| const history = useHistory(); | ||
| const location = useLocation(); | ||
| const [movie, setMovie] = useState({}); | ||
| const [genres, setGenres] = useState([]); |
| movie.genre_ids.includes(parseInt(genreId)) | ||
| ); | ||
| setMovieList(filtered); | ||
| }); |
There was a problem hiding this comment.
if you hit inter with an empty input will broke the code.
use
.catch((err) => {
console.log(err);
});
| return ( | ||
| <> | ||
| <Navbar bg="dark" variant="dark" expand="sm"> | ||
| <Navbar.Brand href="#home" className="text-danger"> |
There was a problem hiding this comment.
This link should take us to the main page
use to with Link, not href
2 participants
This is the first version of our movie app !!
github pages : https://evanhameed99.github.io/my-movie-app/