product-preview-card-component-main ( HTML , CSS)

Community feedback

• 

  Lucas 👾•104,400

  @correlucasPosted about 2 years ago

  👾Hi Abdallah , congrats on completing this challenge!

  I've just opened your live site and I can say that you did a great job putting everything together! There's some tips to improve your solution:

  The approach you've used to center this card vertically is not the best way, because using margins you don't have much control over the component when it scales. My suggestion is that you do this alignment with flexbox using the body as a reference for the container.

  The first thing you need to do is to remove the margins used to align it, then apply min-height: 100vh to make the body height size becomes 100% of the screen height, this way you make sure that whatever the situation the child element (the container) align the body with display: flex and align-items: center / justify-items: center.

  REMOVE MARGINS

  @media (min-width: 700px) and (max-width: 5000px)
  div.contener {
      width: 100%;
      height: 100%;
      align-items: center;
      justify-content: center;
      display: grid;
      /* margin: 20% 20%; */
  }
  

  ADD MIN-HEIGHT: 100VH

  @media (min-width: 700px) and (max-width: 5000px)
  body {
      min-height: 100vh;
      /* width: 100%; */
      /* height: 100%; */
      background-color: hsl(30, 38%, 92%);
      display: flex;
      justify-content: center;
      display: flex;
      align-items: center;
  }
  
  

  ✌️ I hope this helps you and happy coding!

  Marked as helpful

  0
• 

  Adriano•38,080

  @AdrianoEscarabotePosted about 2 years ago

  Hello Abdallah Ahmed, how are you?

  Welcome to the front-end mentor community.

  I really liked the result of your project, but I have some tips that I think you will like:

  1- Document should have one main landmark, you could have put all the content inside the main tag click here

  2- Page should contain a level-one heading, you could change p.head to h1 click here

  I noticed that the content was not aligned and there was a scrollbar on the page to fix this we can do the following:

  removed these:

  div.contener { /* width: 100%; */ /* height: 100%; */ /* margin: 20% 20%; */ }

  I added:

  body { width: 100%; min height: 100vh; }

  * { margin: 0; padding: 0; box-sizing:border-box; }

  This code up here will set margin and padding to 0 for all page elements always use it helps a lot.

  Always prefer to use flex-box or grid, to center a content!

  the rest is great!

  Hope it helps...👍

  Marked as helpful

  0