React Product Preview Card Component

Community feedback

• 

  P

  Jairo Valderrama•420

  @jairovgPosted over 1 year ago

  Hi @vaqueraoscar0, congrats on your solution; here are some comments that might help you to improve it:

  Accessibility and semantics

  • Take a look at how you can improve the component structure using semantic elements. Here is a blog post from freeCodeCamp that may help you.
  • You're setting the image base on some logic done in your JS code; even if it's not wrong, as it gets the goal, you can achieve the same result natively with the <picture> element. Here is the documentation from MDN.
  • Try to use the font sizes using rem units. You'll make the site more accessible to people who need it.
  • Try adding an alt text for the perfume describing the image, not just with product, as this will be the text used by the assistive technology.
  • Think if the cart icon is adding something to your content, if so, leave its alt text, otherwise convert the image to a decorative one or remove its alt text or handling as a background image.
  • This is your heading map:

  React App
  
  1 Gabrielle Essence Eau De Parfum
  5 $149.99
  5 Add To Cart
  

  Think about headings as a table of content of your site, they need to have a hierarchy. Here you can read more about heading structure.

  The structure you're using injects an a11y issue: Heading levels should only increase by one. Here you can read more info about this issue.

  Styles

  • Take a look at xs devices, 320px, like iPhone SE. There is an issue with your component at that breakpoint.
  • I noticed you're trying to add a hover effect to the entire card. That's a good idea, but there is an issue with the implementation. With your current code, if you try to add the click handler to your <button>, if the user clicks outside this element, probably it won't take the action unless you add a click handler to the card also. You need to expand the button click zone, and it may be achieved using a pseudo-element.
  • Consider also adding a cursor pointer on hover and when the card has the cursor hover, change the button state to hover also.

  I hope you find it useful. I'm happy to take another look at your solution if you make some other changes.

  0

  ---

  

  Oscar Vaquera•270

  @vaqueraoscar0Posted over 1 year ago

  @jairovg Thank You! I Appreciate the Feedback

  0