Frontend-Mentor|Social-Links-Profile

Community feedback

• 

  T

  Grace•31,230

  @grace-snowPosted 9 months ago

  Lots of useful learnings to glean from this I think:

  1. I think you need to get a clearer understanding of padding vs margin. This card should have padding on all sides - that is internal spacing to stop it's children hitting the card edges. The elements within the card only need vertical margins. Here is a post that may help you understand margin and padding
  2. Think through html carefully and keep the structure as simple as possible. You don't need that .box div at all. And the list of links should be in a list element.
  3. The name is a heading on this, but h2 is the wrong heading level. It is acting as the page heading, so should be a h1.
  4. The links should not have divs inside. Style the link itself!
  5. The image should have an explicit width and height. Not a percentage width.
  6. Why have you added overflow-wrap: break-word;??? That will cause words to break in strange places.
  7. The links must not have an explicit width. They should be width 100%. I expect you will want to make the links display block or flex (anchors are display inline by default).
  8. Remember it is better for performance to link fonts in the html head and avoid CSS @imports.

  Marked as helpful

  1
• 

  Daniel 🛸•44,270

  @danielmrz-devPosted 9 months ago

  Hello @a-ahmed151!

  Your project looks great!

  I have one suggestion for you to improve it even more:

  • Using margin is not the best option to center an element. Here's a very efficient (and better) way to place an element in the middle of the page both vertically and horizontally:

  📌 Apply this to the body (in order to work properly, don't use position or margins):

  body {
      min-height: 100vh;
      display: flex;  /* it works with grid too  */
      justify-content: center;
      align-items: center;
  }
  

  I hope it helps!

  Other than that, great job!

  Marked as helpful

  1