Product preview card component solution

Community feedback

• 

  P

  Jairo Valderrama•420

  @jairovgPosted over 1 year ago

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

  About fonts

  • Double check the font link you're using, as it imports 400 as the default weight only. Check this version which is yours, vs this other one which includes all the fonts used in the challenge.
  • You'd like to add this two more lines, for performance:

  <link rel="preconnect" href="https://fonts.googleapis.com">
  <link rel="preconnect" href="https://fonts.gstatic.com" crossorigin="">
  

  • Your font families don't have a fallback option, this is a good practice, in case somethinkg happens and the font is not downloaded, i.e: font-family: 'Montserrat', sans-serif;.

  Accessibility

  • I'm wondering why you're using vw to define the font size. Consider using rem for this, it could be setting the parent font size as 100% to let the user define the default value, 16px by default, and then calculate the font size you want for a particular element. i.e.: If you want an element with 14px by default, then its value would be 0.875rem.
  • Good choice using picture to define the image at different breakpoints. As the card image is a content image, consider adding an alt text to the img element so assistive technology can read the image description loud, otherwise, the image would be considered as a decorative one, injecting an accessibility issue.
  • You have multiple a11y issues; you can use tools like axe dev tools if you're not using them yet.

  Regarding your questions/doubts

  ...I couldn't make it responsive until I switched to vw

  The problems you have here are mainly related to the way you're using flex in your component and setting sizes using viewport units instead of letting flex-basis to define the correct size; here is a CSS-tricks blog post related to the use of flex that may help you to solve the issues you have.

  ...I also had trouble with the image, as it wouldn't fill its half of the container until I set a smaller min-width

  Try setting this to the img element instead of setting its width and min-width with viewport units:

  img {
  max-width: 100%;
  height: 100%;
  object-fit: cover;
  }
  

  These properties set the image to be responsive. Also, check how to define the item to the 50% of its parent. Check out the blog post above.

  Others

  • Be careful about abusing the use of viewport units, here is an interesting video about some problems with these units.
  • Think about the title structure you have in your component, should PERFUME be the main heading? and a subheading Gabrielle Essence Eau De Parfum? Probably there is not a correct answer and it may sense your structure, but if not, which other way may be used to define the component title?

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

  Marked as helpful

  1