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

Project Music Releases - Elina Eriksson Hult #103

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ElinaEH
Copy link

@ElinaEH ElinaEH commented Oct 12, 2024

@JennieDalgren JennieDalgren self-assigned this Oct 17, 2024
Copy link

@JennieDalgren JennieDalgren left a comment

Choose a reason for hiding this comment

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

Really good job with this project! You're meeting all requirements and it looks like you've understood how to use props in a good way already.

Component Structure: You have properly divided the components. Each element like the album cover, album name, and artist names has its own component, making the code modular and easy to manage.
Album and Artist Links: The AlbumName and ArtistName components include external URLs that correctly link to the album and artist pages on Spotify using target="_blank" to open in a new tab.
Display of Artists: The ArtistName component handles the display of artist names with commas in between, which is exactly what the requirements ask for.
Props Usage: You've utilized props well to pass data such as URLs, names, and cover images to the components.

Stay golden! ⭐


### The Problem

Describe how you approached to problem, and what tools and techniques you used to solve it. How did you plan? What technologies did you use? If you had more time, what would be next?
This week was a bit overwhelming at first for me. I was a bit confused with all the files already existing in the project and how to grasp everything. I started with making a folder including all the components that I wanted. Then I created the first component and made sure it was working. Then followed along with the rest of the components. Once that was settled I moved on to styling in CSS and applied the hover effect etc. I used google, stack overflow and chatgbpt to help me when I approached problems. If I had more time I would continue with the stretch goals and add a header/title to the site.

Choose a reason for hiding this comment

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

Good reflection! 🌷

<a href={url} target="_blank" rel="noopener noreferrer" className="artist-name">
{name}
</a>
{!isLast && ", "}

Choose a reason for hiding this comment

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

neat ⭐

@@ -10,6 +10,7 @@
"preview": "vite preview"
},
"dependencies": {
"prop-types": "^15.8.1",

Choose a reason for hiding this comment

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

Why installing prop-types if not using them?
suggestion to delete this dependency.

key={artist.id}
name={artist.name}
url={artist.external_urls.spotify}
isLast={index === album.artists.length - 1}

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants