Skip to content

Conversation

@Demijuls
Copy link

Please include a link to your Figma design and a Netlify link.

Copy link

@Npahlfer Npahlfer left a comment

Choose a reason for hiding this comment

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

export const IconsSocMedia = () => {
return (
<SocialWrapper>
<IconWithLink

Choose a reason for hiding this comment

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

Here you can loop out the social media links like this:

const soMe = [
  {
     link: 'https://github.com/Demijuls?tab=repositories',
     image: '/icons/Btn - github.svg'
   },
   ...
]

// And then in the jsx:
{soMe.map((item, index) => (
  <IconWithLink
      image={item.image}
      link={item.link}
  />
))

Then if you add more, you dont have to add any more elements. Or if you need to change something, you only need to change in one place! :)

altText="preview picture for a project"
tags={data.tags}
about={data.about}
reverse={index % 2 === 1}

Choose a reason for hiding this comment

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

Nice use of modulo! :)

background-color: white;
}

@media (prefers-reduced-motion: reduce) {

Choose a reason for hiding this comment

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

Nice touch with turning off all the motion, when the user prefers it!


${media.mobile} {
p {
font-size: 18px !important;

Choose a reason for hiding this comment

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

the mobile breakpoint is not needed here as the tablet is max-width so it will cover the media as well :)

@Npahlfer
Copy link

Overall good! Would have been nice to see some use of the theme file to reduce repetition of colors and fonts. Some repetition so think about how to keep it DRY through the use of components with props and looping out content :)

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