Product preview card component main

Community feedback

• 

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  Hi Sef, well done in completing your second challenge!

  About your question, height is generally not needed, but because you're using background-image for the image, then yes, you would definitely need height in the mobile view if there isn't another container right next to it (to give it the height) or if there is no content itself inside the container. For good practice, I would suggest that you use img instead because background-image is meant for an image used as a background, mainly as decoration. Since this is a product image, it means this is important content and not background decoration at all. You'd always want the main content to be part of the HTML, as this can help screen reader users.

  If you want to try using a responsive img instead where you can put two source images inside an img, you can use this:

  // HTML
      
  <img alt="Glass perfume bottle placed against a leafy background" 
       srcset="images/image-product-desktop.jpg 600w, images/image-product-mobile.jpg 686w" 
       sizes="(max-width: 900px) 686px, 600px"
       src="images/image-product-desktop.jpg" >
  
  // CSS:
  
  .product {
  // remove all background properties 
  }
  
  img {
      max-width: 100%;
      height: 100%;
      object-fit: cover;
  }
  

  Hope this answers your question!

  Marked as helpful

  2

  ---

  

  Sef Palima•60

  @sef1210Posted about 2 years ago

  @elaineleung wow thank you for this that's the main reason why i used background image coz i dont have any idea on how will change the image in the image element. Thank you.

  1

  

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  @sef1210 No worries, I've been there too! Glad to help 😊

  0
• 

  Ebere Ndukwu•180

  @eby-coderPosted about 2 years ago

  .product {
      background-image: url(../images/image-product-mobile.jpg);
      background-size: 20rem;
      background-repeat: no-repeat;
      height: 13.9rem;
      border-top-right-radius: 10px;
      border-bottom-left-radius: 0;
  }
  

  I think I can help you out here. Firstly, I noticed that you set the images as background images. I won't recommend doing this as the images were not set to be background images. So, instead of doing this, I'll recommend this method.

  <img src="image-product-desktop.jpg" class="desktop-img">
   <img src="image-product-mobile.jpg" class="mobile-img">
  

  .mobile-img {
          display: none;
  }
  
  .desktop-img {
        display: block;
        width: 50%;
  }
  
  @media screen and (max-width: 900px){
       .mobile-img {
           width: 100%;
           display: block;
           height: 50%;
       }
  
       .desktop-img {
           display: none;
       }
  }
  

  Putting it this way would make it appear better and not cause possible problems in your page.

  In addition, if you set a border-radius for the card and the images you put overflow and cover the border-radius you set, you can simply use the overflow property in CSS as so:

  .main {
       overflow: hidden;
  }
  

  This would make the overflowing to be hidden. Use this instead of re-adding the border-bottom-left-radius or border-top-right-radius to your image.

  I hope this helps.

  Marked as helpful

  0

  ---

  

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  @eby-coder Hi Ebere, I agree about not using background images, and while toggling images in CSS with display: none can be done, you can also try using responsive images with img or picture, which is way less code and less complicated once you know how to write the attribute values.

  0

  

  Sef Palima•60

  @sef1210Posted about 2 years ago

  @eby-coder Thank you so much for this highly appreciated. I learned a lot and i will use this as reference in the future :)

  0

  

  Ebere Ndukwu•180

  @eby-coderPosted about 2 years ago

  @elaineleung Oh I didn't know this. Thanks

  1