product-preview-card

Community feedback

• 

  バレンタイン 😈•64,170

  @VCaramesPosted almost 2 years ago

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

  • The image’s alt tag description needs to be improved upon to better describe what it is. You will want to assume that you are describing the image to a someone.

  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 an example of how it works: EXAMPLE

  Syntax:

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

  More Info:📚

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

  • The only heading in this component, is the name of the perfume; “Gabrielle Essence Eau De Parfum” . The rest of the text should be wrapped in a paragraph 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 sr-only 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

  1

  ---

  

  Sumaj Coronel•90

  @sumajestad44Posted almost 2 years ago

  @vcarames Very helpful information, thank you!!

  1
• 

  Hassia Issah•50,650

  @HassiaiPosted almost 2 years ago

  Replace <div class="flex-container"> with the main tag,<h4> with <p> and remove the <h3> within the button to fix the accessibility issue. for more on semantic html visit https://web.dev/learn/html/semantic-html/

  There is no need to give the body a max-width value in the descktop design and a width and height value in the mobile design.

  To center .main-container on the page using flexbox, add min-heigh:100vh; align-items: center; to flex-container.

  Use rem or em as unit for the padding, margin, width and preferably rem for the font-size for more on this watch this https://youtu.be/N5wpD9Ov_To

  Hope am helpful. HAPPY CODING

  Marked as helpful

  1

  ---

  

  Sumaj Coronel•90

  @sumajestad44Posted almost 2 years ago

  @Hassiai Hi, thank you very much for the feedback. But i did what you told me to center the .main-container but it didn't work

  0

  

  Hassia Issah•50,650

  @HassiaiPosted almost 2 years ago

  @sumajestad44 apply the centering to the body instead of flex-container, i taught flex-container was acting as the body.

  To center .main-container page, add min-height:100vh; display: flex; align-items: center: justify-content: center; or min-height:100vh; display: grid place-items: center to the body. And remove justify-content from the flex-container.

  Hope am helpful.

  Marked as helpful

  0

  

  Sumaj Coronel•90

  @sumajestad44Posted almost 2 years ago

  @Hassiai Very helpful, it works perfectly, thank you again!

  1