Design comparison
Solution retrospective
This is my first attempt. I made sure to add rounded corners in correct spots when resizing. The mobile mode does look a bit warped, BUT it does respond to smaller screen sizes. I ended up using 'srcset', which was pretty cool.
The font sizes might be a little off. Let me know if there is anything odd about the CSS, I tried to condense it. Also, just let me know in general what can improve, I am still pretty new at this.
Community feedback
- @elaineleungPosted over 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 theimg
selector. For the images in this challenge, I think you can just useobject-fit: contain
(which makes sure the whole image is viewed within the parent container and its width contained within the sides). I also would adddisplay:block
to theimg
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, removegrid-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 andpicture
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
toproduct-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 helpful2@WhaleMentalistPosted over 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@elaineleungPosted over 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 useobject-fit: contain
, you're telling the browser to not stretch the image while making sure the entire image is contained within the parent. Removing the1fr 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 thegrid-template-row
unless there's a need to. Hope this helps a bit!Marked as helpful0 - @codedforfreePosted over 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 helpful1@WhaleMentalistPosted over 2 years ago@codedforfree Taking your suggestion now and looking into variables! And I'll try messing around with using.
0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord