Oscar and Regina's Project Movies #325
Conversation
| } | ||
|
|
||
| return ( | ||
| <section className="details-page"> |
There was a problem hiding this comment.
To display a background as well, you could add this code inside of the section tag for "details-page"
style={{ backgroundImage: url(https://image.tmdb.org/t/p/w1280${details.backdrop_path})` }}`
There was a problem hiding this comment.
Checklist
- Am I able to understand the code easily?
Yes, I few more comments would be great as well! - Does the code have lots of duplicates? Could it be tidied up?
In the components the is quite clean, maybe if you would have used the same CSS design approach it might have generated less code. - Does it work or does anything look broken?
All of it works, great job! - Does it follow the general and blue requirements?
Yes! Minor fixes would be needed to match the design fully, like rating and background image in the ShowMovie-component.
Great job Oscar and Regina!
/Annika and Sara
| import React from 'react' | ||
| import { Link } from 'react-router-dom' | ||
|
|
||
| export const PopularList = ({ movies }) => { |
There was a problem hiding this comment.
The fetch is working perfectly, great job!
| fetch(`https://api.themoviedb.org/3/movie/${id}?api_key=5b17ba21606be74c35e11124051ec659`) | ||
| .then((res) => res.json()) | ||
| .then((json) => { setDetails(json) }) | ||
| .catch((error) => alert(error, 'error')) |
There was a problem hiding this comment.
Good idea to add a catch in case of errors! :)
| <button type="button" className="back-link" onClick={goBack}> Back to movie list </button> | ||
| <img className="solo-movie-poster" src={`https://image.tmdb.org/t/p/w342/${details.poster_path}`} alt="poster" /> | ||
| <h2>{details.title}</h2> | ||
| <p>{details.overview}</p> |
There was a problem hiding this comment.
In order to not have the text spread out over the full width of the page, you could add max-width to the p-element.
| .then((json) => { setDetails(json) }) | ||
| .catch((error) => alert(error, 'error')) | ||
| .finally(() => console.log('movie-details')) | ||
| }, [id]) |
There was a problem hiding this comment.
Just out of curiosity, did you also get an error about not adding movie-details as a dependency?
| /*ShowMovie*/ | ||
| .back-link { | ||
| font-size: 10px; | ||
| right: 75%; |
There was a problem hiding this comment.
What does these properties do? Are they positioning the button?
| } | ||
|
|
||
| /*ShowMovie*/ | ||
| .back-link { |
There was a problem hiding this comment.
If you want to style the button more, this is how we styled our button. We used flexbox to position it. :)
.backButton {
background-color: transparent;
color: white;
cursor: pointer;
font-weight: 700;
border: 5px white;
padding: 8px 15px;
opacity: 0.7;
display: flex;
flex-direction: row;
gap: 10px;
}
| width: 30vh; | ||
| align-self: center | ||
| } | ||
| } |
There was a problem hiding this comment.
We see that you used flexbox for the mobile version and grid for the other devices in the all-movies container. Interesting approach, we didn't think about it, wondering if the code might look cleaner when the same approach is used on all devices for one component?
| @@ -5,9 +5,130 @@ body { | |||
| sans-serif; | |||
There was a problem hiding this comment.
Daniel recommended splitting the CSS into separate files for each component. We found that it was easier for us to work asynchronously when having separate CSS files.
https://oscarreginaprojectmovies.netlify.app