Gabriels-Support-Team / Organization-Music-Explorer

Starter code for the Music Playlist Explorer project
0 stars 0 forks source link

Samy Alihamad - Week 1 Project Feedback #1

Open samyalihamad opened 5 months ago

samyalihamad commented 5 months ago

Overall, the project looks great and you were able to complete about 95% of the core functionality plus most stretch goals. This is right in-line with what your project should look like. My only major feedback is I noticed some of the project's UI implementation deviated away from the original project designs (I listed them below under the Core Functionality section). Just something to keep in mind is that as your projects get larger and more complex even small deviations in the original design could lead to larger refactors later on. If the requirements need to change be sure to bring that up as early as possible. Again your project looks great and the deviations from the original design were minor but this is just something I wanted to call out early on.

Core Functionality

  1. The like playlist feature should have a heart icon according to the requirements.
  2. The modal layout for playlist coverage image, title, and creator does not match the images in the project requirements.
  3. The Featured button's entire row responds to the the button click event. See https://pxl.cl/52T42

Stretch Functionality

  1. I noticed the "Add New Playlist" has both a "Submit" and "Add Song" button and both appear to do the same thing. Is there any difference?
  2. When creating a new playlist the first time selecting the playlist the modal opens with the correct information. However, if I select a different playlist, close the modal, and select the playlist I created the modal shows the previous playlist. See https://pxl.cl/52QBv
  3. One suggestion would be to separate the New Playlist form from the Add Song form.
  4. The Edit playlist button is either disable or not hooked up.

Code

  1. These changes are perfectly fine for your first project but as a discussion topic what are some changes do you think could make your form submission logic more secure?
  2. In your code you reference the playlist container with const gridContainer = document.querySelector('.grid-container');. Is there an alternative approach you could use that would prevent any conflicts in the future as your application grows?
  3. The toggleLike() function is defined within your submitForm() function. One quick improvement you could make is have it defined outside of your submitForm() to make it reusable outside of your core playlist functionality.
  4. The code indentation is not consistent across which may sound like a minor issue but consistent formatting helps with code readability.
  5. A minor suggestion to improve readability would be to group all functions together and group the core execution code like populateGridContainer(); together.
Capn05 commented 5 months ago

Some key takeaways for next weeks project: adhere exactly to the listed requirements in terms of user Interface make sure to format code for readability Extract functions that could be used in multiple spots Group functions together for readability