Design comparison
Solution retrospective
Checkout my code and give a like and feedback. Thank you in advance.
Community feedback
- @AdrianoEscarabotePosted about 2 years ago
Hi Kevin, how are you? I really liked the result of your project, but I have some tips that I think you will enjoy:
A good practice to center content is using
grid
orflex-box
, avoid using margin or padding to make placements, use only in the latter case! we can do it like this:Flex-box:
body { display: flex; align-items: center; justify-content: center; flex-direction: column; min height: 100vh; }
GRID
body { display: grid; min height: 100vh; place-content: center; }
The rest is great!
I hope it helps... 👍
Marked as helpful1 - @IamAbhiDevPosted over 1 year ago
Hey Kevin, How are you? First of all, thanks for helping me with my first-ever project and giving some great advice to boost my confidence! I liked your result but here's a suggestion that I would like to provide to you:
Using the
background-image
property for your product image may not be the best practice. This is a meaningful image and product images should always have a proper description with the alt attribute. Also, background images can be less efficient compared to theimg
tag and you'll have to write more CSS code to hide or show each image.What I recommend you to do is:
Use the
<picture>
element to showcase the image of your product. Only the image you need for your screen size/resolution gets downloaded in this case.Here's an example of how you can do it:
<picture> <source srcset="./images/image-product-mobile.jpg" media="(max-width: 37.5rem)"> <img src="./images/image-product-desktop.jpg" alt="product-preview"> </picture>
Hope this helps!
Marked as helpful0
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