Product Preview Card Component

Community feedback

• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted about 2 years ago

  Hi @Vinzz34 👋, good job completing this challenge, and welcome to the Frontend Mentor Community! 🎉

  In addition to the previous comments. I have some suggestions you might consider to improve your code:

  • In this challenge, you should not use the background property to set the image because this image has semantic meaning. Use the CSS background property if the image is not part of the content.

  • You can use a <picture> tag when you need to change an image in different viewports. Using this tag will prevent the browser from loading both images, saving bandwidth and preventing you from utilizing a media query in your CSS file to modify the image.

  Example:

  <picture>
     <source media="(max-width: 50em)" srcset="./images/image-product-mobile.jpg">
     <img src="./images/image-product-desktop.jpg" alt="your alt text">
  </picture>
  

  • You could use the <del> tag to display the old price:

  <del class="wrong">
     <span class="sr-only">Old price: </span>$169.99
  </del>
  

  Note that I added the <span> with the sr-only class to the del element, this will provide more information about what your old price is about.

  The sr-only class is a class that you can add to hide content visually but is only visible to screen-readers.

  More information here.

  • The cart icon is for decoration purposes only, so it can be hidden from screen-readers by adding aria-hidden="true" and leaving its alt attribute empty:

  <img class="cart-icon" src="./images/icon-cart.svg" alt aria-hidden="true">
  

  I hope those tips will help you! 👍

  Good job, and happy coding! 😁

  Marked as helpful

  2

  ---

  

  Vinzz•280

  @Vinzz34Posted about 2 years ago

  @MelvinAguilar thanks a lot will definitely be using these tips for sure!

  1
• 

  Adeola Ganiu•1,320

  @DeolabestPosted about 2 years ago

  Hey @Vinzz34, Congratulations on completing this challenge!

  Here is my feedback:

  • It's not a good practice to use px to set font-size. Instead use rem units, they are great since they adapt better to the font-size the user will set in the browser settings. You can convert from px to rem here: https://pixelsconverter.com/px-to-rem.

  • Use <main> instead of a simple <div> to improve the semantics and accessibility on the page. Remember that every page should have a <main> block and that <div> doesn't have any semantic meaning.

  Keep doing a good job!

  Marked as helpful

  1

  ---

  

  Vinzz•280

  @Vinzz34Posted about 2 years ago

  @Deolabest thanks for the feedback i mostly used rem for font-size and also used main in the beginning but later changed it to a div thanks for the feedback though will use these tips

  0