Design comparison
SolutionDesign
Solution retrospective
What are you most proud of, and what would you do differently next time?
I am most proud of my figuring out how to srcset pictures in the html.
What challenges did you encounter, and how did you overcome them?I had a lot of issues getting the white content to line up with the desktop image. To solve the whitespace issue, I set a height for the desktop image at the 1440px breakpoint.
What specific areas of your project would you like help with?Any constructive feedback is welcome.
Community feedback
- @MattPahutaPosted 5 months ago
- Good use of semantic HTML, however, the card's picture and content should not be separate sections. They are elements of the same product card, so better to make the card an individual section (within your main tag) or simply a div and use child divs to create your column/row layout.
- Similarly, there's no need to set a width on your main tag. The card component should be a child of the main tag. Set a max-width and use a media query on the card component to achieve the desired size of the component.
- Grace Snow wrote an excellent article on how she would approach writing this project's HTML. I highly recommend checking it out.
- Also, great use of the picture tag to handle the card image but you don't (and shouldn't) need to set a height. The object-fit property is worth using here as well to prevent image distortion.
- Related to the image, instead of setting border-radius properties directly, set it on the card (parent) along with overflow: hidden. You can then remove border-radius properties on child elements.
- You're styling on ID selectors in a few places. This should be avoided as ID selectors can result in very high specificities and create styling conflicts that are difficult to troubleshoot. Use classes or element selectors instead.
- Don't set a max-width or width on the body element. If you need a way to contain the card component, use a wrapper div. For this project, you could use the main element or, better yet, avoid one altogether. Using flexbox or grid display properties on your body element would work to keep the card component aligned and centered.
- Your CSS is nicely concise but you ought to use a modern CSS reset for this project (and any in the future). Josh Comeau and Andy Bell both have excellent resets you can incorporate into your CSS file (or use as a separate file).
- Media queries should be set in rem/em, not px. Here's a great resource
- You're missing hover and focus states on the button.
- Overall, very nice work here. With some relatively minor revisions to your code, you'll have a much closer match to the design comp. Cheers!
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