perfume card-component

Community feedback

• 

  David•8,000

  @DavidMorgadePosted over 2 years ago

  Hello Angel, congrats on finishing this challenge on FrontEndMentor!, here are a few tips that can help you for next projects!

  1 - To center a simple component like this want there is an easy way with just a few lines of code, instead of giving your main any margin, give your body a heigth: 100vh and then a display: flex, flex-direction: column, justify-content: center (to center vertically) and align-items: center (for horizontal centering).

  2 - For your image, the easies way to use two image for the same img tag and switching them depending on the size, is to use the <picture> tag and the source tag, you can wrap your img with this tag and depending on the device screen, display diferent images, for example:

  <picture>
       <source media="(min-width:660px)" srcset="./images/image-product-desktop.jpg" >
       <img src="./images/image-product-mobile.jpg"  alt="Product picture 
  </picture>
  

  In this case, for 0 to 660px you will have the mobile version, and for 660px+ you will have the desktop version.

  Hope my feedback helps you, good work and keep going!

  Marked as helpful

  2

  ---

  

  Angel Iran•20

  @angeliranPosted over 2 years ago

  @DavidMorgade Thank you very much for your advice

  0
• 

  Elaine•11,400

  @elaineleungPosted over 2 years ago

  Hi Angel Iran, good job putting this together! It looks like your image is slightly distorted, but no worries, that can be fixed with object-fit by adding object-fit: cover to your img element.

  I see you using a breakpoint of 376px; the tricky thing is your component overflows off the screen until the breakpoint changes. Since the component is about 600px in desktop view, I suggest a breakpoint slightly higher than that (e.g., 620px). The other problem I see is that, you only have the mobile image in the HTML. You can use the picture element in your HTML by adding these lines around your <img>:

  <picture>
      <source srcset="./images/image-product-desktop.jpg" media="(min-width: 620px)">
      <img src="./images/image-product-mobile.jpg" alt="">
   </picture>
  

  Lastly, to center everything, use display:grid on your body selector, like this:

  body {
    display: grid;
    place-content: center;
    min-height: 100vh;
  }
  

  Well done!

  Marked as helpful

  2

  ---

  

  Angel Iran•20

  @angeliranPosted over 2 years ago

  @elaineleung Thank you very much for your advice

  1
• 

  Ogiji Max•140

  @MaxiTeddyPosted over 2 years ago

  Thats not the image you should for the desktop view, check your folder and change the image

  0