Grid Layout w/ Flexbox for Product Card

Community feedback

• 

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  Hi Spencer Daniel, well done on this second challenge! I think you did many things well, and good job on using the picture element. It looks like you learned a lot in this challenge and also handled a lot of problem solving 🙂

  About the image being warped, what you can do is to use object-fit on the img selector. For the images in this challenge, I think you can just use object-fit: contain (which makes sure the whole image is viewed within the parent container and its width contained within the sides). I also would add display:block to the img container, but better yet is to add a reset rule at the top of your stylesheet for all images, like this:

  img,
  picture,
  video,
  canvas,
  svg {
    display: block;
    max-width: 100%;
  }
  

  Once you add the object-fit, you'll see the image shift, and there should be a lot of white space; to fix that and remove all the white space, remove grid-template-rows: 1fr 1fr; in the .product-card for that media query. I'd also change the media query breakpoint to something a bit higher, like 620px in both the CSS and picture element; since the width of your desktop component is 600px, you'd want a bit a white space before it switches.

  Lastly, I'd add margin:1rem to product-card so that in the mobile view the sides of the card wouldn't touch the browser sides.

  Great work here, and keep coding! 🙂

  Marked as helpful

  2

  ---

  

  Spencer Daniel•50

  @WhaleMentalistPosted about 2 years ago

  @elaineleung Appreciate the feedback. Didn't realize there was an object-fit property! I implemented your suggestions and it definitely fixed the warping. I am still scratching my head on why removing 'grid-template-rows' after adding in 'object-fit' seemed to fix the whitespace. Strange, I'll have to look into grids to read some more.

  1

  

  Elaine•11,400

  @elaineleungPosted about 2 years ago

  @WhaleMentalist Glad to help! The reason for removing 1fr 1r for the rows is that, the 1fr for each row means they are equal in height, and before the object fit, the browser simply stretched the image to fill the space to make sure the image and the text boxes are the same in height. When you use object-fit: contain, you're telling the browser to not stretch the image while making sure the entire image is contained within the parent. Removing the 1fr 1fr means you no longer tell the browser how tall the rows should be, but you allow the browser to make the decision itself based on the content of the containers. It's usually better to not write in the grid-template-row unless there's a need to. Hope this helps a bit!

  Marked as helpful

  0
• 

  Plinsinga•310

  @codedforfreePosted about 2 years ago

  I think it looks quite good if you’re new to this. If I where you I would start by using some css variables for some of the basics like the fonts and colors.

  Also take a look at mobile styles. Maybe you can remove the grid styles and set it to block. Now you’re using grid to set a default behaviour.

  Keep up the good work 👍

  Marked as helpful

  1

  ---

  

  Spencer Daniel•50

  @WhaleMentalistPosted about 2 years ago

  @codedforfree Taking your suggestion now and looking into variables! And I'll try messing around with using.

  0