Product preview card using flexbox

Community feedback

• 

  Izzat Razab•70

  @izzatrazabPosted over 1 year ago

  Nice work on completing the challenge. 👍

  I have some suggestions on your code that may help you.

  • Set height of html and body to 100%, why ? this will make centering any contents much easier and will avoid extra white space at the bottom of the page or when zooming (though this does not happens in your case). To do that:

  html{
  height: 100%;✅
  }
  body {
  min-height: 100%;✅
  background-color: hsl(30, 38%, 92%);
  }
  

  • To center the card: Instead of using margin which is not responsive and akward, I would suggest using display:flex;. Now that we already set height of html and body element to 100% before, there will be no awkward slightly offset position because they are covering you viewport 100% (in desktop view, your card is a little bit off to the top):

  body {
  min-height: 100%;
  display: flex;✅
  justify-content: center;✅
  align-items: center;✅
  background-color: hsl(30, 38%, 92%);
  }
  .pricecard { 
  width: 90%;
  /* margin: 2em auto 2em auto; */❌
  background-color:  hsl(0, 0%, 100%);
  border-radius: 20px;
  max-width: 600px;
  }
  @media (min-width:600px) {
  .pricecard {
  display: flex;
  max-width: 500px;
  /* margin-top: 10em; */❌
  }
  

  • Landmark main element: I see that you did not use main element. This is actually bad practice as using landmark helps screen reader users easily navigate your website. But why? Look here briefly: HTML5 landmark elements are used to improve navigation. Try evaluate your deployed website here. Though you don't have to stress out too much on this because it will make sense to you the more you practice. I would do this instead:

  <main class="pricecard">✅
  <div class="pricecard">❌
  ...
  </div>❌
  </main>✅
  

  I downloaded your code and tried all the above locally and it works fine. If you found this helpfull, please upvote this comment as it will help me improve. That's all from me, adios.

  0