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

Code Review #27

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6bebcf1
Added new Components
mayaatmeta Jun 13, 2024
5d03bde
Added App styling
mayaatmeta Jun 13, 2024
065f2a2
Added Modal styling
mayaatmeta Jun 13, 2024
d63990f
Added MovieCard styling
mayaatmeta Jun 13, 2024
e5d78ad
Added MovieCard Component
mayaatmeta Jun 13, 2024
026f9f2
Added MovieList styling
mayaatmeta Jun 13, 2024
883bfe1
Added MovieList Component
mayaatmeta Jun 13, 2024
7916f26
Added Search Component
mayaatmeta Jun 13, 2024
c3d439a
Added api key
mayaatmeta Jun 13, 2024
091f138
hid api key
mayaatmeta Jun 13, 2024
7472cab
edited file name
mayaatmeta Jun 13, 2024
147c02e
Added Modal Component
mayaatmeta Jun 13, 2024
2f3a39b
Working Modal
mayaatmeta Jun 13, 2024
c6f4bf5
Modal updated with runtime
mayaatmeta Jun 13, 2024
97585ba
Modal with runtime ✅
mayaatmeta Jun 13, 2024
c9ad1e3
Modal Component✅
mayaatmeta Jun 13, 2024
116414d
Added like and watched features
mayaatmeta Jun 13, 2024
677bb79
Filtering and Sorting feature✅
mayaatmeta Jun 14, 2024
e7fe6ff
Footer added
mayaatmeta Jun 14, 2024
80ebbb4
Working sidebar, no movies added.
mayaatmeta Jun 14, 2024
04a9369
Adding liked songs to side bar feature ✅
mayaatmeta Jun 14, 2024
d7d123c
Adding watched movies to sidebar feature✅
mayaatmeta Jun 14, 2024
ef5479d
Updated modal with poster background and embedded youtube trailer
mayaatmeta Jun 15, 2024
0ef6526
final css changes made
mayaatmeta Jun 15, 2024
58adaa7
Update README.md
mayaajike Jun 15, 2024
7b94bce
Update README.md
mayaajike Jun 15, 2024
6247b27
style updates
mayaatmeta Jun 20, 2024
e7f81f1
Merge branch 'main' of https://github.com/mayaajike/flixster-starter
mayaatmeta Jun 20, 2024
7d53211
Update README.md
mayaajike Jun 21, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 66 additions & 16 deletions src/App.css
Original file line number Diff line number Diff line change
@@ -1,28 +1,78 @@
.App {
#root {
max-width: 1900px;
margin: 0 auto;
/* padding: 2rem; */
text-align: center;
background-color: #f5f5f5;
}

.App-header {
background-color: #282c34;
.header {
border: 2px solid indigo;
width: 100%;
background-color: indigo;
display: fixed;
}

.title {
color: white;
}
form{
display: flex;
flex-direction: row;
align-items: center;
justify-content: space-evenly;
justify-content: flex-start;
margin-left: 25px;
}

#search-bar {
padding: 10px;
width: 500px;
margin-right: 3px;

}

#search-button {
background-color: #B19CD9;
padding: 10px;
border: 2px solid darkmagenta;
border-radius: 5px;
}

#search-button:hover,
#nowPlayingButton:hover,
#searchTabButton:hover {
color: white;
padding: 20px;
background-color: darkmagenta;
border: 2px solid #B19CD9;
border-radius: 5px;
}

@media (max-width: 600px) {
.movie-card {
width: 100%;
}
#nowPlayingButton {
background-color: #B19CD9;
padding: 10px;
border: 2px solid darkmagenta;
border-radius: 5px;
margin-right: 10px;
}

#searchTabButton {
background-color: #B19CD9;

Choose a reason for hiding this comment

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

we reuse background-color: #B19CD9; quite a few times here, in the future let's try to capture repeated styles in a separate class so we can avoid repetition

padding: 10px;
border: 2px solid darkmagenta;
border-radius: 5px;
}

.search-bar {
flex-direction: column;
gap: 10px;

.searchForm {
display: none;
}

@media all and (max-width: 630px) {
#search-bar {
width: 300px;
}
}

.search-bar form {
flex-direction: column;
@media all and (max-width: 400px) {
#search-bar {
width: 100px;
}
}
110 changes: 103 additions & 7 deletions src/App.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,106 @@
import { useState } from 'react'
import './App.css'
import React from 'react';
import MovieList from './MovieList';
import Search from './Search';
import { useEffect, useState } from 'react';
import Modal from './Modal';
import './App.css';
import './Modal.css';

const App = () => {
<div className="App">

</div>
function App() {
const [movies, setMovies] = useState([]);
const [page, setPage] = useState(1);
const [searchValue, setSearchValue] = useState('');
const [modal, setModal] = useState(false);
const [movieID, setMovieID] = useState('');

const getMovies = async () => {
const url = `https://api.themoviedb.org/3/movie/now_playing?language=en-US&page=${page}&api_key=0d7613c1b95dbc61f3dd491c8f802475`;
const options = {
method: 'GET',
headers: {
accept: 'application/json',
Authorization: 'Bearer eyJhbGciOiJIUzI1NiJ9.eyJhdWQiOiIwZDc2MTNjMWI5NWRiYzYxZjNkZDQ5MWM4ZjgwMjQ3NSIsInN1YiI6IjY2Njc2NmJhODAyN2M0OWNmYjk5ZmJiYyIsInNjb3BlcyI6WyJhcGlfcmVhZCJdLCJ2ZXJzaW9uIjoxfQ.0iAwLYCfoeheb8A8_TWhHNrdDn2P1x3LL-d0_fGs6BA'
}
};

const response = await fetch(url, options);
const jsonResponse = await response.json();
if (page === 1){
setMovies(jsonResponse.results)
} else {
setMovies(prevMovies => [...prevMovies, ...jsonResponse.results])
}

}

const searchMovies = async () => {
const result = searchValue.replace(/ +/g, '+');
const url = `https://api.themoviedb.org/3/search/movie?query=${result}&api_key=0d7613c1b95dbc61f3dd491c8f802475`;
const options = {
method: 'GET',
headers: {
accept: 'application/json',
Authorization: 'Bearer eyJhbGciOiJIUzI1NiJ9.eyJhdWQiOiIwZDc2MTNjMWI5NWRiYzYxZjNkZDQ5MWM4ZjgwMjQ3NSIsInN1YiI6IjY2Njc2NmJhODAyN2M0OWNmYjk5ZmJiYyIsInNjb3BlcyI6WyJhcGlfcmVhZCJdLCJ2ZXJzaW9uIjoxfQ.0iAwLYCfoeheb8A8_TWhHNrdDn2P1x3LL-d0_fGs6BA'
}
};
const response = await fetch(url, options);
const jsonResponse = await response.json();
const results = jsonResponse.results.filter((movie) => {
return movie.title && movie.title.toLowerCase().includes(searchValue.toLowerCase());
})
if (results.length === 0){
return "Try again, No results found :)"
}else {
setMovies(results);
}

Copy link

@hanluc hanluc Jun 20, 2024

Choose a reason for hiding this comment

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

Overall looks good to me! The website looks great to me and functioning well!

Just a few nit on spacing and newlines. I noticed that a lot of functions have series of consecutive newlines, can we delete the newlines to make file structure cleaner?


}

useEffect(() => {
getMovies();
}, [page])

useEffect(() => {
searchMovies()
}, [searchValue])

const loadMore = () => {
setPage(page => page + 1);
};
// setModal(!modal);
Copy link

Choose a reason for hiding this comment

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

Why do we need this line?

const movieClick = (movieId) => {
setMovieID(movieId);
}


return (
<>
<header className="header">

Choose a reason for hiding this comment

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

this should have an indent. Somehow format is important for code readebility. things like indentation, empty line,...

<h1 className="title">Flixster</h1>
<Search className="searchForm" searchValue={searchValue} setSearchValue={setSearchValue}/>

<button id="nowPlayingButton" onClick={() => {
const form = document.getElementsByClassName("searchForm")[0];
const loadButton = document.getElementById('load-more');
form.style.display = 'none';
loadButton.style.display = 'grid';
loadButton.style.marginLeft = '40%';
getMovies();
}}>Now Playing</button>

<button id="searchTabButton" onClick={() => {
const form = document.getElementsByClassName("searchForm")[0];
const loadButton = document.getElementById('load-more');
form.style.display = 'block';
loadButton.style.display = 'none';
searchMovies();
}}>Search</button>
</header>
<Modal data={movies} movieID={movieID}/>
<MovieList data={movies} loadMore={loadMore} handleMovieClick={movieClick}/>
</>
)
}

export default App
export default App;

Choose a reason for hiding this comment

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

you can do export default component App(){...} more to read on React flow component syntax https://fburl.com/6hxctx6w

31 changes: 31 additions & 0 deletions src/Modal.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
.movieModal{
display: none;
position: fixed;
z-index: 1;
left: 0;
top: 0;
width: 100%;
height: 100%;
overflow: hidden;
background-color: rgba(225, 225, 225, 0.5);
/* margin-top: 15px; */
Copy link

Choose a reason for hiding this comment

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

remove this unused line?

}

.modalContent {
display: grid;
position: relative;
margin: 10% auto;
padding: 20px;
width: 63%;
height: 75%;
border: 2px solid black;
border-radius: 10px;
background-color: black;
justify-content: center;
color: white;
}

.body-no-scroll {

Choose a reason for hiding this comment

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

let's try to keep the styles name format consistent, bodyNoScroll

overflow: hidden;
height: 100%;
}
Empty file added src/Modal.jsx
Empty file.
Empty file added src/MoviCard.css
Empty file.
Empty file added src/MovieCard.jsx
Empty file.
Empty file added src/MovieList.css
Empty file.
Empty file added src/MovieList.jsx
Empty file.
Empty file added src/Search.jsx
Empty file.