@DylandeBruijn
Posted
Hiya @AngeATT,
Good job on this solution, it looks great!
I'm happy to give you some friendly constructive feedback:
-
I think you forgot to set the
font-family. Try setting it on thebodyelement. Don't forget some elements have a differentfont-family. -
Try putting
min-height: 100vhon yourbodyelement. This makes it so the element takes up a minimum height of100vhwhile still allowing the content to grow without overflow issues. -
You have
height: autoon your.containerelement. This is not needed asblockelements already haveheight: autoby default. If you add more content the element grows automatically. -
I suggest using an
imgtag for your image instead of abackground-image. You can useobject-position: centerandobject-fit: coverwhen using images to achieve the same result when using background images. -
Try comparing the design to your solution again (especially the mobile view) and try to achieve the same using media queries.
I hope you find my feedback helpful, as always I would appreciate it if you could mark my comment as helpful when it was!
Let me know if you have more questions and I'll try to answer them.
Marked as helpful
@AngeATT
Posted
Thank you very much, @DylandeBruijn, for your suggestions. I will look forward to taking into consideration every recommendation you made.
@AngeATT
Posted
I made some changes, can you take a look? @DylandeBruijn
@DylandeBruijn
Posted
Hiya @AngeATT,
I'm happy to see that you take the time and effort to improve your solution, that's a great mindset! I checked out your code and it looks a lot better than before! Responsive as well, props!
A couple finishing touches:
-
At the moment the image scales well but you can see some distortion. This is because the image has a fixed
height: 200pxbut awidth: 100%. So thewidthis fluid but theheightisn't. The image tries to fit in that box without respecting aspect ratio so to say. You could fix this by settingobject-fit: coveron the image. Which tells the element to scale its content to fit the aspect ratio of the containing element while completely covering the containing element. I notice I made a mistake in my previous comment, so it should beobject-fitand notobject-size, sorry about that! Another way you could fix this issue is by keeping thewidth: 100%and removing the fixedheight: 200px. The image then can scale freely without distortion! -
Preparation time block is missing
border-radius. -
You could change the first table cells (
td) in the table rows (tr) tothto make it a little more semantic. Like<th>Carbs</th>. You can also directly put the text in thetdwithout wrapping it in aspan. -
If you want to make your CSS values more reusable I recommend reading about CSS custom properties!
Once again great job @AngeATT, excited to see more of your progress!
Marked as helpful
@AngeATT
Posted
Hello @DylandeBruijn, changed done, can you look again
