-
Notifications
You must be signed in to change notification settings - Fork 197
feat: Flixter - Pull Request to base repo #64
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
base: main
Are you sure you want to change the base?
Conversation
feat: styled movieList & MovieCard
…e. Basic formatting
added search functionality to filter movies based on search and clear added switch function to switch between now playing and search results
…clear button - Implemented MovieModal component with escape key and overlay click-to-close behavior - Added responsive modal styling with Netflix-style animation - Integrated backdrop, metadata (genres, rating, runtime), and overview sections -Updated clear button to also clear the movies being displayed when clicked
- Added heart and eye icons to each movie tile for favoriting and marking as watched - Toggled visual state changes on icon clicks (filled vs outlined) - Implemented state management for favorites and watched lists - Built sidebar with navigation to Home, Favorites, and Watched pages - Each view filters and displays the appropriate movie set in grid layout
style: added design finishing touches
Updated README for submission
| @@ -0,0 +1,190 @@ | |||
| import { useEffect, useState } from 'react'; | |||
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.
Nice use of useEffect and useState
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.
Thank you!
| Submitted by: Jason Chimdinma Jason | ||
|
|
||
| Currently, two official plugins are available: | ||
| Estimated time spent: 36 hours spent in total |
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 improvement on speed and productivity from last assignment!
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.
Thank you for the advice!
| - [X] Website is deployed via Render. | ||
| - [X] **VIDEO WALKTHROUGH SPECIAL INSTRUCTIONS**: For ease of grading, please use the deployed version of your website when creating your walkthrough. | ||
|
|
||
| #### STRETCH FEATURES |
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.
Awesome job on completing all required and stretch goals.
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.
Thank you!
Co-authored-by: Ameya Gawde <ameya918@gmail.com>
ishanbalar
left a comment
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.
Overall good improvements. I am happy with the current progress Few nit changes required,
- decouple react component to make it more reusable.
- Remove commented lines
- format the code
- style values should be as per the industry standard instead of random numbers
| display: flex; | ||
| flex-direction: row; | ||
| align-items: center; | ||
| /* padding-left: 15rem */ |
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.
Please dont commit commented line of code
src/App.jsx
Outdated
| <MovieCard key={movie.id} movie={movie} /> | ||
| ))} | ||
| </div> | ||
| <MovieList /> |
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.
Glad to see that you have created a modularized component
src/App.jsx
Outdated
| <div className="App"> | ||
| <header className="App-header"> | ||
| <h1>Flixster</h1> | ||
| inp |
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.
Not sure if this is typo ?
src/MovieList.jsx
Outdated
| console.error("Error fetching movies:", error); | ||
| setError("Failed to fetch movies. Please try again later."); | ||
| setLoading(false); |
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.
Try to implement error page and show user friendly error message on the browser for exceptions
src/MovieList.css
Outdated
| @@ -1,12 +1,17 @@ | |||
| .movie-list-container { | |||
| padding: 20px; | |||
| padding: 11rem 3.5rem; | |||
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.
You should use these constant values from this series of numbers i.e. 2, 4, 8, 12,16, 20, 24 to follow better practice
src/MovieList.jsx
Outdated
| import './MovieList.css'; | ||
|
|
||
| const MovieList = ({ searchQuery, view, onViewToggle }) => { | ||
| const MovieList = ({ searchQuery, view, sortBy, onViewToggle }) => { |
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.
From this commit you are making this MovieList component more tightly coupled. AS discussed in our 1:1 you should modularize the component so it can be re-use anywhere
| onToggleFavorite={toggleFavorite} | ||
| onToggleWatched={toggleWatched} | ||
| isMovieFavorite={isMovieFavorite} | ||
| isMovieWatched={isMovieWatched} |
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.
Try not to bubble up these events and restrict these events upto its own component to scale the application better.
| <div className="no-results"> | ||
| {currentPage === 'favorites' | ||
| ? "You haven't added any favorites yet" | ||
| ? "you havent added any favorites yet" |
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.
nit: haven't v/s havent
| const detailsResponse = await fetch(detailsUrl); | ||
| const detailsData = await detailsResponse.json(); |
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.
Will this following one line works instead of 2 constants ?
const detailsData = await fetch(detailsUrl).json();
| /* background-color: #282c34; */ | ||
| /* display: flex; | ||
| flex-direction: row; | ||
| align-items: center; | ||
| justify-content: space-between; */ |
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.
Commented lines lead to un-readable code, Please avoid having them
https://www.loom.com/share/4b1b3fd186ca4d41a64356b68e177e92?sid=e26d51d4-9960-43cc-839e-0f3480604cc4
📽️ Flixster – Movie Discovery Web App
Submitted by: Jason Chimdinma Jason
Total Time Spent: 36 hours
Deployed App: My Site
Overview:
Flixster is a responsive movie discovery platform that allows users to browse, search, and sort movies using live data from The Movie Database (TMDB) API. Built with React, the app delivers an interactive experience with features like modals, accessibility, and sorting.