Product preview card component using CSS grid

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted about 2 years ago

  Hi

  It’s extremely important to learn html - in particular the importance of choosing the correct elements for the content. That means

  • use landmark elements - this should have a main at least
  • do not use lists for non list content. There is nothing that would be construed as a list from this design
  • use the picture element for responsive image swapping by the browser, not two img tags
  • use del element for the old price.
  • screenreaders are not told when text is styled with a line through and not always told even when the del element is used. So it is best to add some visually hidden text in a span or pseudo element to say that's the old price.
  • add to cart is an action not navigation, so it must be a button element not an anchor
  • no ids are needed on this
  • use the cart icon supplied with the challenge
  • the product image is meaningful content so needs proper alt text. This must be a brief description of what the image looks like, a human readable description, as if I can't see the image at all. It is not a hyphenated string like code or a class name
  • place classes on what you need to style

  Marked as helpful

  1

  ---

  

  T

  Grace•31,270

  @grace-snowPosted about 2 years ago

  On my mobile this looks broken and the css is why

  1. Never use width 100vw. That makes the screen wider than available space if/when a scrollbar is present
  2. Never limit the height of the body. Use min-height instead
  3. Never give cards like this a fixed width. Use max width instead so the component can go narrower if it needs to (ie. On small screens or when font sizes are enlarged)
  4. You should be working mobile first
  5. You can set border radius on the whole component and use overflow hidden so the corners of the img gets cropped to the radius. This is simpler than setting individual corner radius at each breakpoint
  6. Just as font size must never be in px, nor should letter spacing. This will break for anyone who changes their base font size. So use em for letter spacing instead
  7. Never style on IDs. It's not what they are for and increases specificity of css when you don't want to
  8. If you use flex / inline-flex on the button you can use flex properties like align items and gap to arrange the button contents instead of margin
  9. Font sizes shouldn't be changing on mobile
  10. A little margin on the component will protect it from hitting the sides of the screen

  Marked as helpful

  1

  

  Aman Rai•160

  @Cypher-amanPosted almost 2 years ago

  @grace-snow Thank you for all the valuable advice you have given me. I will surely take your advice and try to improve and not repeat these mistakes again.

  0