Skip to content

Mini challenge 1#1

Open
alfredo-pacheco wants to merge 3 commits into
masterfrom
mini-challenge-1
Open

Mini challenge 1#1
alfredo-pacheco wants to merge 3 commits into
masterfrom
mini-challenge-1

Conversation

@alfredo-pacheco

Copy link
Copy Markdown
Owner

Mini-challenge-1 for React Bootcamp.

@PacoMojica PacoMojica left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Awesome deliverable, keep the good work 👍

Comment on lines +6 to +11
return (
<MainContainer>
<Header>This is the header</Header>
<Content>{children}</Content>
</MainContainer>
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good, I like this, it is simple but expressive

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Thanks :)

Comment on lines +22 to +28
<VideoCard key={item.id.videoId}>
<VideoPreview src={item.snippet.thumbnails.medium.url} />
<VideoContent>
<VideoTitle>{item.snippet.title}</VideoTitle>
<VideoDescription>{item.snippet.description}</VideoDescription>
</VideoContent>
</VideoCard>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The way you keep the VideoCard components separate helps with readability, you can go one step further and move all the things related to the VideoCard to a separate file, this way you can test the component easily

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

OK Thanks!

Comment on lines +39 to +47
export {
Title,
VideosContainer,
VideoCard,
VideoPreview,
VideoContent,
VideoTitle,
VideoDescription,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great use of the styled components

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Thanks!

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