repsonsive product card using CSS Flexbox

Community feedback

• 

  hitmorecode•7,540

  @hitmorecode

  Posted almost 2 years ago

  In general your page looks good. I see you used three media queries. You could have done it with just one

  • When creating a page, go with mobile first approach. This is actually the default setup of a page.
  • When you have a class inside another class, you don't have to write it like that in css. For example you have class .previous-price and it's inside .product-price, you don't have to do it like this

  .product-price .previous-price {
    font-family: 0.2em;
    text-decoration: line-through;
    color: var(--color-dark-grayish-blue);
  }
  
  ## You could have done it like this, because previous-price is your target and css knows where it is.
  .previous-price {
    font-family: 0.2em;
    text-decoration: line-through;
    color: var(--color-dark-grayish-blue);
  }
  

  If you have something like this

  <div class="main">
    <p>Lorem ipsum lorem ipsum</p>
  </div>
  
  <div class="second-main">
    <p>Lorem ipsum lorem ipsum</p>
  </div>
  

  Because the p tags have no class and if you want to target a specific p tag, you cannot tell css to target a p, it will target all p tags. In this case you can do it like this

  .main p {
    color: green;
  }
  
  .second-main p {
    color: yellow;
  }
  

  If you give a p tag a class or id, then you can just target that class or id

  You usde margin to place the card in the middle of the page, you don't have to user margin, use flexbox or grid.

  ## Lines that are commented out can be removed.
  body {
  /*   padding-inline: 1rem; */
    display: flex; /* add these lines */
    min-height: 100vh;
    justify-content: center;
    align-items: center;
    flex-direction: column;
  }
  
  .product-card {
    max-width: 65rem;
    height: 45rem;
    /* aspect-ratio: 4 / 3; */
  /*   margin: 6rem auto 2rem; */
    background-color: var(--color-white);
    box-shadow: 2px 2px 5px gray;
    display: flex;
    border-radius: 1rem;
    overflow: hidden;
  }
  

  21

  Marked as helpful
• 

  Jennifer Souza•340

  @zoedarkweather

  Posted almost 2 years ago

  Hi, looks pretty good overall. I agree with the above comment that mobile-first is the way! I'd also point out you can use the picture element in your html to switch out the image for the correct screen size. Here's the mdn reference page on that if it might help: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/picture. Best!

  11

  Marked as helpful