Social Links Profile

Community feedback

• 

  sunnyegg•80

  @sunnyeggPosted 22 days ago

  Hi! First of all, good job on completing the challenge! There are some things that I noticed when I looked at your site and the code.

  1. The website is fine when I open it on Desktop Screen, but when I downsize the screen to Mobile, the card got really tiny. I think you can just remove the width and the height on your card class. Or you can set the maximum of the card's size using max-width and max-height attributes.

  2. Remove unused codes in your CSS file.

  Cheers!

  Marked as helpful

  1

  ---

  

  devindubin•60

  @devindubinPosted 22 days ago

  Thank you for commenting! @sunnyegg

  I just pushed an update that fixed the mobile presentation, there was a bug in my css media query.

  0
• 

  Dani Bachour•40

  @dbachour86Posted 22 days ago

  Nice work, but isn't it better to use h2, h3, and ul tags? You are using a lot of p tags.

  1

  ---

  

  devindubin•60

  @devindubinPosted 22 days ago

  Thank you for commenting! @dbachour86

  I chose not to use other header tags because apart from the user name the sizes looked fairly similar to me and didn't require such a distinction. Apart from default sizing, I am unfamiliar with the benefits of using header tags (apart from a single h1) as opposed to setting classes. Maybe I could have saved some code when setting font-weights that way? I hadn't considered it so thank you for the tip!

  As for the links, I chose not to nest them in a ul tag because they appeared similar enough to the other text (which was also centered) to keep them in the same container as everything else rather than have li elements inside a ul element. Next challenge I will try your method and see if I notice any major differences.

  Thank you!

  0