Responsive Product Card Component using CSS

Community feedback

• 

  P

  Kamran Kiani•1,280

  @kaamiikPosted 25 days ago

  Hi. Congrats for doing this challenge. I noticed some points I wanna mention:

  • In your picture use min-width in rem instead of px.

  ---

  • There should be only one h1 in your page because It is your page title and we only have one title. and the price I think is not a heading and you can simply use p tag for price number.

  ---

  • Try to use a proper CSS reset at the start of your CSS style. Andy Bell and Josh Comeau both have a good one. You can simply search on the internet to find them.

  ---

  • Your font-size and max-width should be in rem unit not px. You can read this article about it and why you should not use px as a font-size.

  ---

  • Never limit your width and height in a container or element or tag that contains text inside. When you limit the width and height of elements containing text, you risk the text being cut off, overflowing, or becoming unreadable, especially on smaller screens or when the text dynamically changes. It's generally better to allow the container to adjust its size based on its content or set a flexible size that can adapt to different screen sizes and text lengths. You only need max-width here because it prevents elements from stretching beyond a certain point, keeping them visually appealing across different screen sizes. It ensures your design remains adaptive and doesn't get too wide on larger screens.

  Marked as helpful

  1
• 

  David Sibrian•140

  @armson45Posted 26 days ago

  Hey there! Maybe you should try do mobile first.

  • In your card-body-price you've put two h1 tags, it makes sense in the title but not as a price, instead use a span or a p tags.

  • The use of the picture is good and the problem with the image is that you have this: .card-img img { width: 100%; height: 100%; } the actual problem is the height, it makes that the image always takes the 100% of the space in the parent available, so when you go in small viewports the image covers all the card so just delete that line or put it in auto but it may generates another problem.

  As I say before, try the mobile first. It's easier to take care of responsive that way.

  Everything else looks good for me .

  Have a nice code! :)

  0