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-9-music-releases #102

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Conversation

erikamolsson
Copy link

@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.

Good job with this project!
You are almost meeting all requirements.

You are using components in a good way, however you havent really passed any props for one component to another. This was kind of the main goal for the week. The easiest way to acheive this without refactor too much of you code would be to pass the data that you need from App.jsx down to MusicGallery. Something like this:
<MusicGallery albums={data.albums.items} />
And in MusicGallery file:

const MusicGallery = ({ albums }) => {
  return (
    <div className="album-grid-container">
      {albums.map((album) => (
        <div key={album.id} className="album-card">
          {/* Rest of the code */}
        </div>
      ))}
    </div>
  );
};
  • Make sure to pass props to a component
  • Make sure to split the artists up adding a comma between them, if more than one artist per album.

Keep up the good work! 💪

Comment on lines 6 to 9
<div className="footer-music">
<p>Made by: Erika Olsson</p>
<p>Technigo / Web Development Boot Camp</p>
</div>

Choose a reason for hiding this comment

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

remember to use semantic elements even though we're working in React (

is better than a
)

Comment on lines 9 to 16
color: #FFFFFF; /* White text */
text-transform: uppercase;
font-size: 4rem;
font-weight: 700;
width: 300px;
height: auto;
border-bottom: #FFF solid 3px;
border-top: #FFF solid 3px;

Choose a reason for hiding this comment

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

for the color, either stick to #FFF or #FFFFFF for consistency

Comment on lines +13 to +18
/* const [albums, setAlbums] = useState([]);

// Load the data directly from the imported JSON file
useEffect(() => {
setAlbums(data); // Directly use the imported data
}, []); */

Choose a reason for hiding this comment

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

Remove unused code or add comments to why you are keeping it.

@@ -24,11 +24,11 @@ npm i && code . && npm run dev

### 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?
I understood the principle of React pretty fast, how to work with components. But thought it was difficult to know how to divid everything up and how many components it should be. But it gets clearer! Have to few components in this project - but realised that to late and let it be, but next time i will divide it into more smaller components.

Choose a reason for hiding this comment

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

Good reflection. It will get clearer and clearer for each time! Also with a bigger project it will feel more "logical" to split it up more.

@erikamolsson erikamolsson marked this pull request as draft November 4, 2024 15:09
@erikamolsson erikamolsson marked this pull request as ready for review November 4, 2024 15:10
@erikamolsson erikamolsson marked this pull request as draft November 4, 2024 15:10
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