-
Notifications
You must be signed in to change notification settings - Fork 198
flixster codereview 3 (file re-org and utils) #55
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
- Rename fetchMovieList() to fetchAndProcessMoviesByIDList() for clarity - Split fetchAndProcessMoviesByIDList() into fetchMovieByID() and processMoviesByID() - Rename fetchSearchMovies() to fetchAndProcessMoviesBySearch() for clarity - Split fetchAndProcessMoviesBySearch into buildMovieSearchURL(), fetchMoviesBySearch(), and processMoviesBySearch() - Refactor sortMovies() to sortMovieOrder() for clarity and replace if/else with switch/case for readability - Update useEffect dependencies [mode, pageCleared] to use switch/case for readability These changes improve code readability and maintainability by clarifying function purposes and enhancing control flow readability.
…ishing Refactoring Movie Fetch Functions and Sorting
… favorites/watched and then back with text in searchText
…}, moved modal component to movielist component
fix - removed testing console logs fix - renamed promises to moviePromises for readability
…ishing Jackmccl/flixster/styling and polishing
…ishing Fixed minimum movie card width to 90px
…or jsx and css Fix - sorting mode now resets when swapping to favorites or watched
Jackmccl/file reorg Fixing sidebar sort mode bug moved api and sorting to utils
| :root { | ||
| --lucida-font: 'Lucida Sans', 'Lucida Sans Regular', 'Lucida Grande', 'Lucida Sans Unicode', Geneva, Verdana, sans-serif; | ||
| --steelgray: #1e1e2f; | ||
| --martinique: #2a2a4c; | ||
| --butterflybush: #505091; | ||
| --wildblueyonder: #8080bc; | ||
| --gallery: #f0f0f0; | ||
| } |
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 css variables here!
| @@ -0,0 +1,66 @@ | |||
| export const fetchMovieByID = async (movieID) => { | |||
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 work organizing helper functions into a util to reduce code clutter :)
| // Takes a list of movie entries (a list of {id: int, movie: {}}) and sorts by vote average property (highest to lowest) | ||
| export const movieEntriesVoteAverageSort = (movieEntries) => { | ||
| movieEntries.sort((left_entry, right_entry) => { | ||
| return right_entry[1].vote_average-left_entry[1].vote_average |
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 and concise :)
|
LGTM |
auroraw9825
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.
Awesome work!
Definitely see very big improvement on code quality, thanks for following up on our comments and you learn so fast!
| const data = await response.json() | ||
| return data | ||
| } catch (error) { | ||
| console.error(error) |
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.
qq" when there is an error fetching movies, is it a blank page?
| let response = null | ||
| response = await fetch(`https://api.themoviedb.org/3/movie/${movieID}/videos?language=en-US`, options) | ||
| if (!response.ok) { | ||
| throw new Error('Failed to fetch videos') |
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.
same question here: wondering what the behavior is when fetching is failed
Summary
Reorganized components to each have their own folder with their jsx and css
Refactored api calls into apiUtils in utils folder
Refactored sorting methods into sortingUtils in utils folder
Testing
Tested search, sorting, favorites, watched, and movie modal components to ensure functionality after refactoring (components that use API calls and sorting in utils) ✅


