Product Card Component with CSS Grid and Flexbox

Community feedback

• 

  hitmorecode•6,680

  @hitmorecodePosted about 1 year ago

  Nice well done. Just a few tips and how to solve the button problem

  • The price problem you could have solved like this. If you add flexbox to prices it will work fine.

  <div class="prices">
      <p class="discounted-price">$149.99</p>
      <p class="before-price">$169.99</p>
  </div>
  

  • If you remove padding: 10px 0 0 10px; from .discounted-price it should solve the problem
  • If you add this to your css, you can remove all paddings

  .product-content {
    padding: 10px;
  }
  

  • You don't have to add the font-family multiple times. Check which font family is the most used and add it on the body. The other(s) font family you can add separate.

  • You should change the <h2> to <h1>

  • I did some changes to your css with some comments

  body {
    background-color: hsl(30, 38%, 92%);
    min-height: 100vh;/* add this to be able to place the card in the middle of the page */
    display: flex;
    justify-content: center;
    align-items: center;
    flex-direction: column;
  }
  
  img {
    border-radius: 2% 0 0 2%;
  }
  
  .product {
    display: grid;
    background-color: #fff;
    max-width: 36.5rem;
    height: 27rem;
  /*   margin: 200px auto; */ /* you can remove this, you already have flexbox on the body */
    display: grid;
    grid-template-columns: 1fr 1fr;
  /*   gap: 15px; */ /* you can remove this */
    border-radius: 2%;
  }
  
  .product-content { /* add this to add padding on the card */
    padding: 20px 10px;
  }
  
  .perfume-title {
  /*   padding: 10px; */ /* you can remove all paddings. this is in the product-content */
  /*   margin-top: 20px; */
    font-family: "montserrat";
    font-weight: 500;
    font-size: 13px;
    color: hsl(228, 12%, 48%);
    margin-bottom: 15px; /* add margint on the bottom */
  }
  
  .product-title {
  /*   padding: 10px; */
    font-size: 34px;
    line-height: 1;
    font-family: "fraunces";
    font-weight: 700;
    color: hsl(212, 21%, 14%);
    margin-bottom: 20px; /* add margint on the bottom */
  }
  
  .product-description {
  /*   padding: 10px; */
    line-height: 1.5;
    font-family: "montserrat";
    font-weight: 500;
    color: hsl(228, 12%, 48%);
    font-size: 14px;
    margin-bottom: 30px; /* add margint on the bottom */
  }
  
  .prices {
    display: flex;
  }
  
  .discounted-price {
  /*   padding: 10px 0 0 10px; */
    font-family: "fraunces";
    font-weight: 700;
    font-size: 30px;
    color: hsl(158, 36%, 37%);
  }
  
  .before-price {
    align-self: center;
    padding: 0 0 0 10px;
    font-family: "montserrat";
    font-weight: 500;
    font-size: 13px;
    color: hsl(228, 12%, 48%);
    text-decoration: line-through;
  }
  
  .btn {
    display: flex;
    justify-content: center;
    align-items: center; 
    width: 100%; /* add this to stretch the button to use the full width */
    font-family: "montserrat";
    font-weight: 700;
    margin-top: 30px;
    padding: 15px;
    color: #fff;
    border-radius: 7px;
    border: none;
    background-color: hsl(158, 36%, 37%);
    transition: 0.3s;
    cursor: pointer;
  }
  
  .cart-icon {
  /*   position: relative; */
    margin-right: 10px;
  }
  

  You still need to make the page responsive, you can easily do this with media query

  It's up to you to chose either you use css grid or flexbox or both. There is no right or wrong. Whatever makes you feel comfortable is what you should use.

  If it's not clear, let me know

  Marked as helpful

  0

  ---

  

  nadezhda-frunza•150

  @nadezhda-frunzaPosted about 1 year ago

  @hitmorecode Thank you so much for this detailed answer! Really appreciate it.

  In general yes, I noticed that I can make my code shorter in some parts, font-family is a great example of that. Will look closely to not repeat these in the future. Completely forgot about vh! Will review the lectures about units.

  Noted the other parts as well, thanks for all the useful comments in the code.

  0