search

Gabriels-Support-Team / flixster-starter

0 stars 0 forks source link

Project 2 Feedback - Samy Alihamad #1

Open samyalihamad opened 3 months ago

samyalihamad commented 3 months ago

Overall

Overall the project looks great! It was easy to use and overall I didn't find any issues navigating around searching and viewing movies. Including the movie trailer in the movie details section was a really nice stretch goal to accomplish; it really made the app a lot more interactive and engaging. Also, I appreciate you reaching out to me during development to confirm your layout designs that were not specified in the requirements; I'm glad to see you actioning on the feedback from your first project.

Core Functionality

  1. I noticed the search functionality has partial matching which is a great addition.
  2. This is minor, but I noticed the Load More button sometimes loads to the right of the movies on the last row (See screenshot: https://pxl.cl/54XFR). For consistency, I would have it load in its own row each time.

    Stretch Functionality

  3. All the stretch goals look really great!
  4. Adding the movie trailers was a nice stretch feature. This might be a limitation of the API but some movies don't have a trailer. In those cases, the YouTube component fails.

Code

  1. If you take a look at the network tab you'll see that that the api call https://api.themoviedb.org/3/genre/movie/list?language=en is continually called (See screenshot: https://pxl.cl/54XFV). How could you ensure that the network call is made only the necessary amount of times? What are the downsides to you as the app developer and the api creator of making unneeded network calls?
  2. This is not a requirement for the project, but what are some options for securing the api keys (i.e. Bearer token)? What are some considerations when storing data in your .env file?
  3. The base url https://api.themoviedb.org/3 is used in multiple places. In real world applications teams may have different base urls for dev, stage, and prod environment. How could you ensure you can handle different environments?
  4. In your fetch calls you are already handling .catch((error) so you could take this time to also display an error message to the user. This is a consideration for future projects.
  5. Most of the fetch calls could also be used elsewhere as your application grows. What's a good approach to ensure you don't repeat code multiple times?
  6. The favorite icon is a common component in any application. In MovieCard.jsx you have the favorie icon is very specific to the MovieCard.jsx. One consideration is to make this into its own functional component where you pass in the onClick event as a parameter the component.
Capn05 commented 3 months ago

Thank you for your detailed feedback!

I've taken note of the key issues you highlighted and have outlined how I plan to address them in this week's project:

Consistent UI Elements: Noted from the 'Load More' button issue, I will ensure consistent placement and behavior of similar UI elements in my next project Optimized API Calls: The issue with repeated API calls in the last project was a great learning point. For the new app, I will make sure to implement controlled fetching

Code Reusability and Maintenance: I will focus on creating reusable components and services right from the start.

Component Design: Inspired by the feedback on the favorite icon, I will design functional components with reusability in mind