product preview card

Community feedback

• 

  バレンタイン 😈•64,170

  @VCaramesPosted almost 2 years ago

  Hey there! 👋 Here are some suggestions to help improve your code:

  • Do not forget ⚠️ to check your FEM report (It provides value information), to see what is incorrect and update your code with it. This should be done immediately after submitting your challenge.

  • The image’s alt tag description needs to be improved upon ⚠️. Assume that you are describing the image to a someone over the phone.

  More Info:📚

  https://www.w3.org/WAI/tutorials/images/

  • This component requires the use of two images 🎑 at different breakpoints ⚠️. The picture element will facilitate this.

  Here is how it looks like implemented: EXAMPLE

  Syntax:

    <picture>
      <source media="(min-width: )" srcset="">
      <img src="" alt="">
    </picture>
  

  More Info:📚

  https://www.w3schools.com/html/html_images_picture.asp

  • Do not uppercase ❌ "perfume" in HTML as it is not accessible friendly. Instead you will want to style it in CSS.

  • The name of the perfume , “Gabrielle Essence Eau De Parfum”, is the most important content in your card so it should be wrapped in a `heading’ element. ⚠️

  • Currently, the old price (169.99) 🏷 is not being properly announced 😢 to screen readers. To fix this, you are going to wrap the the price in a del element and inside it you will add a span element with an visually-hidden class that will state something like “The previous price was…” and use CSS to make it only visible to screen readers.

  More Info:📚

  Del Element

  If you have any questions or need further clarification, feel free to reach out to me.

  Happy Coding! 🎆🎊🪅

  Marked as helpful

  0
• 

  Hassia Issah•50,650

  @HassiaiPosted almost 2 years ago

  There is no need to give the body a class inordre to style it.

  Replace <div class="product-body-wrapper"> with the main tag,<div class="product-content-infoheader"> with <p>, <div class="product-content-header"> with <h1> and <div class="product-content-body"> with <p> to fix the accessibility issues. click here for more on web-accessibility and semantic html

  there is no need fo a height in .product-body-wrapper, the height is causing the overflow of the content. the padding value for .product-content is a responsive replacement of that.

  To center .product-body-wrapper on the page using flexbox, replace the height in the body with min-height: 100vh.

  Use relative units like rem or em as unit for the padding, margin, width values and preferably rem for the font-size values, instead of using px which is an absolute unit. For more on CSS units Click here

  You forgot to add a media query for the mobile design, in the media query give .product-body-wrapper a max-width value and flex-direction: column. Give .product-content and .product-image wrapper a width of 100%.

  Hope am helpful.

  Well done for completing this challenge. HAPPY CODING

  Marked as helpful

  0

  ---

  

  Gaurav chauhan•50

  @ravenbirdb1bPosted almost 2 years ago

  @Hassiai Thanks for the feedback!

  0