Product preview card component

Community feedback

• 

  Amal Karim ✅•1,290

  @amalkarimPosted almost 2 years ago

  Hi Ted!

  When I visit your solution page using a smartphone in landscape mode, some of the content are hidden and I can't scroll down to see them. I believe this is not because of overflow: hidden; of the section element, but rather because of this style:

  html, body {
      height: 100%;
  }
  

  When the height of the browser is not enough to show all of its content, some of the content become hidden, because the browser height is not allowed to grow taller than 100%. It can be solved by changing height: 100%; to min-height: 100%; or min-height: 100vh;.

  Other than that, I think it's quite a nice solution. But if you want to take the extra step, here is what I suggest:

  • Add alt attribute to the main image. Because it's an important and meaningful image, it must have alternative text to it. Please refer to this article about image accessibility if you want to know more about it.

  That's it. Hope this helps. Happy coding!

  Marked as helpful

  0
• 

  バレンタイン 😈•64,190

  @VCaramesPosted almost 2 years ago

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

  • It is best practice to have separate files for your HTML and CSS code to make it easier to maintain and organize.

  • To not only improve your HTML code but to also identify the main content of you page, you will want to wrap your entire component inside the main element.

  More Info:📚

  MDN Main Element

  • The alt tag description for the image needs to be improved upon. You want to describe what the image is; they need to be readable. Assume you’re describing the image to someone.

  More Info:📚

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

  • 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

  • Your attribution should be wrapped in a footer element instead of a div.

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

  Happy Coding!🎄🎁

  Marked as helpful

  0