Skip to content
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

Week 8: Annika, Bridget, and Frida's Trending Movies #309

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

beemailley
Copy link

No description provided.

Copy link

@AndreaHedstrom AndreaHedstrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gooood job! Everything, both the page in browser and the code, looks clean and works perfect. It's easy to follow the code and you have a good structure - as I wrote I like that you have the CSS in different files. That makes it easy for me and others, who haven't worked with the project from the beginning, to understand and find what we're looking for.

Go Tigers! <3


return (
<div className="single-movie-wrapper" style={{ backgroundImage: `linear-gradient(rgba(0, 0, 0, 0) 70%, rgb(0, 0, 0) 100%), url(http://image.tmdb.org/t/p/w1280/${movieDetails.backdrop_path})` }}>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice with the styling for the backgroundImage here, new for me!

import './styling/singlemovie.css';
import './styling/footer.css'
import './styling/trendinglist.css'
import './styling/notfound.css'
import { App } from './App';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impressing and clear to follow and read when you have the CSS for different components in different files!

.back-button:hover {
transform: scale(1.1);
opacity: 1;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the button! And fun that something happens when you hover!

@@ -0,0 +1,2 @@
export const BASE_URL = `https://api.themoviedb.org/3/trending/movie/day?api_key=${process.env.REACT_APP_KEY}`
export const SINGLE_MOVIE = (id) => `https://api.themoviedb.org/3/movie/${id}?api_key=${process.env.REACT_APP_KEY}`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the "process.env.REACT..." doing? :) This is you API key, right? Where are the one million numbers etc that I always got as a key?

Anyway, like that you got it in a seperate file - can imagine it's nice to have it this way in the future when we're doing larger project with a lot more components etc, easy to find.

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.

4 participants