-
Notifications
You must be signed in to change notification settings - Fork 417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Yu's movie-site project #315
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job Yu, you managed well even though there was some back and forth with the group constallation and you did it on time!
You should not feel discouraged at all, well done, just some minor tweaks that I noticed in the code. Stay on it!
</div> | ||
<ul className="navlinkWrapper"> | ||
<li className="link"> | ||
<NavLink to="/">Home</NavLink> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey!, maybe change this to what it actually is (and says on the page when you open that part) - popular movies?
<div className="popular-tv-container"> | ||
{tvList.map((singleTv) => { | ||
return ( | ||
<div className="tv-wrapper"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might I suggest making it easier for you if you use the same classnames on these divs as in the movie parts? That way you don't have to apply css duplicates
return ( | ||
<div className="tvDetailsWrapper"> | ||
<img | ||
className="tv-imge-backdrop" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo you might want to correct :)
<div className="title"> | ||
<h2>{details.title}</h2> | ||
<div className="rating"> | ||
<span>{(Math.round(details.vote_average * 10) / 10)}⭐</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that some titles, since they have not been released yet, have not gotten any votes on rating yet.
One way to solve that problem, just not displaying 0 and then a star, could be to do a ternary operator that if line 49 = 0 return string "N/A" or something like "Not enough votes"
the link to this project: https://yourmoviesiteby-ym.netlify.app/