Product Preview Card Component

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted 18 days ago

  Hi, well done on this, it looks pretty good. I have some suggestions that I hope are helpful.

  • you don't need to repeat the Google font preconnect links in the html head. One set is enough.
  • strike through text isn't consistently announced to screen readers. So include some visually hidden text in a span that says "Old price" or similar so that those users know which is the old/original/pre-sale price and which is the new one.
  • the main product image needs an alt description that describes the image (the distinguishing features of the product, what the image conveys or looks like). It mustn't just repeat the name only which is already in the heading.
  • you can set border radius on the card and use overflow hidden on it to crop out the overflowing child corners instead of repeating border radius on its children.
  • really a wrapping element (like the body in this case) should have padding instead of the card having margin. This is because once this card would be dropped into place on a real site it is unlikely it would want margin on all sides. Think of component and page layout/Sublayout styles separately and this mental model helps later when you work on larger projects.
  • check you definitely want to use rem for the padding on the text container. You may find that makes the text too narrow if someone enlarges their default text size (especially on smaller screens). Px or a min() css function that includes viewport width units may work better for that.
  • you can remove display block from the button as you are changing it to flex lower down.

  I'm not sure what problem you encountered specifically with the icon or what you mean when you say "found a way to make it work". Happy to advise if you explain what the issue was.

  It looks like you've changed the code in that svg a little as it now has invalid height and width attributes. They cannot include px units in those attributes. You shouldn't have to change them at all tbh.

  Marked as helpful

  1

  ---

  

  Michelle Chong•120

  @mdchongPosted 18 days ago

  @grace-snow Hi Grace and thank you so much for the feedback! About the icon, I had a hard time as the SVG file that was provided would not render on the page. I double-checked if they were on the same file, opened it on Illustrator to check what the matter was but it was just fine, I tried displaying it as block, and changed the height and width, but could not figure out anything.. ): So I decided to use a different icon provided by Google. I would appreciate it if you could advise me about the SVG file. Other than that, I will make the changes to improve the web!

  0

  

  T

  Grace•31,270

  @grace-snowPosted 18 days ago

  @mdchong The svg is white. So did you place it on an element with a background colour? Because it wouldn't show up if just opened on a screen with a white body background (white on white)

  0

  

  Michelle Chong•120

  @mdchongPosted 18 days ago

  @grace-snow Yes I did place it on an element with a background color but did not work ): But I think I got it figured. I downloaded the starter file again and moved the icon to my existing file and now it's working! Thank you so much (:

  1
• 

  P

  Kamran Kiani•1,280

  @kaamiikPosted 18 days ago

  Hi. Congratulation for doing this challenge. I really liked it. Your HTML and CSS code is great. Specially I liked using hgroup in your code. Knowing where to use html semantic tags is very valuable. About the scss, Kevin Powell has a good template for scss in his github. One with Vite and one with the Astro. You can use them too.

  Fantastic job! Keep up the great work and keep pushing forward

  Marked as helpful

  1

  ---

  

  Michelle Chong•120

  @mdchongPosted 18 days ago

  @kaamiik Hi Kamran! Thank you so much for the feedback! Kevin Powell is on my notes and definitely will check it out!

  1