Social-links-profile

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  Here's what I notice

  • all content should be contained within landmarks. Wrap this component in a main to get into the habit of including one in every page you ever do to hold the main page content.
  • this is only one card. There is no reason for the extra divs inside it. Keep html as simple as possible.
  • heading tags are really important elements. They must be used appropriately and in the right order. Don't skip heading levels. They should read like a table of contents in a long document, showing the relationships between different sections of content. This card only has one heading. The London location makes no sense as a heading for all the other content in the card, it's just a paragraph.
  • personally I think this list of links should be structured in an actual list.
  • remove the target attributes when you are not using them.
  • the links dont all need different classes. They share styling so should all have the same class.
  • when importing fonts from Google, copy all of the code provided not just the last line.
  • get into the habit of including a **full ** modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
  • don't limit the height of elements that contain text, including the body. Instead of height 100vh, use min-height: 100svh. That tells the browser its allowed to be taller than the viewport when it needs to.
  • make sure the body is a flex column not row if using that.
  • font weights are not set in px. That's invalid.
  • font size should never be in px. Use rem. https://fedmentor.dev/posts/font-size-px/
  • I'm not sure youre understanding the difference between padding and margin looking at this. The card and links should have padding. Everything within the card should have margin for vertical spacing (or gap used on parent flex/grid elements). It's probably worth reading this https://fedmentor.dev/posts/padding-margin/
  • don't set white space nowrap on the paragraphs! That can lead to bad overflow bugs.
  • line height is usually unitless like 1.5.
  • the links must not have an explicit width. All they need is display block and/or width 100%. They should have a little padding too.
  • the card should have a max width in rem.
  • if all fonts are the same you should be able to define that font family once high up in the dom and other child elements will inherit it. Better than having to define it loads of times.

  Marked as helpful

  2

  ---

  

  RichardTagoe•70

  @RichardTagoePosted about 2 months ago

  Thank you for your suggestion i will make sure to apply them in my following challenges. Please do well to look at my other works and give me your comments. Thank you @grace-snow

  0
• 

  Rohan Pawar•30

  @rohanpawar680Posted about 2 months ago

  This comment was deleted about 1 month ago

  0

  ---

  

  T

  Grace•31,270

  @grace-snowPosted about 2 months ago

  @rohanpawar680 please stop leaving these meaningless comments everywhere. Look for meaningful feedback or questions to ask.

  0