Product preview card component

Community feedback

• 

  Huy Phan•2,300

  @huyphan2210Posted 28 days ago

  Hi, @naveenkkannan I checked out your solution and I have some thoughts:

  • The issue with your <picture> element having extra height is due to it being slightly taller than the <img> inside. You can see this difference more clearly by inspecting it in your browser's DevTools.
  • This extra height is common with inline-level elements like<img> that align to the text baseline by default, leaving a small gap below. This can make the container (<picture>) appear taller than the <img>.

  Here are two ways to fix this:

  • Set vertical-align on <img>

  Adding vertical-align: top or vertical-align: bottom to the <img> removes the extra space below:

  picture img {
    vertical-align: top; /* Or use `bottom` */
    // Your styles
  }
  

  • Set <picture> to display: flex

  Applying display: flex on <picture> removes the gap since Flexbox doesn’t have the baseline alignment issue by default.

  Either approach should eliminate the extra space, with the flex method also giving you alignment control if needed!

  Hope this helps!

  Marked as helpful

  0

  ---

  

  Naveen K•50

  @naveenkkannanPosted 27 days ago

  Thank you for taking the time to review my solution and provide such detailed feedback! I implemented the method you suggested, and setting the vertical-align on <img> worked perfectly to eliminate the extra height. Your guidance was really helpful @huyphan2210

  1
• 

  P

  Lo-Deck•2,790

  @Lo-DeckPosted 28 days ago

  Hi well done for this challenge,

  It's better to use em or rem instead of px. FreeCodeCamp. FreeCodeCamp.

  I don't understand why you set a lot of @media (min-width:768px) , usually you set it just once. So It's hard to read your code, keep it clear, after it's a mess when you need to debug it or someone else try to read your code.

  Hope to be helpful.

  Marked as helpful

  0

  ---

  

  Naveen K•50

  @naveenkkannanPosted 27 days ago

  Thank you for your feedback! I appreciate the suggestion to use em or rem instead of px for better scalability. I’ll also work on consolidating the multiple @media (min-width: 768px) rules to improve code readability. Your insights are very helpful!@Lo-Deck

  0