@brasspetals
Posted
Hi, Guilherme! Congrats on submitting your first solution! 🎉 Excellent job on the image coloring, which is a big struggle for most people on this challenge. 🙌
A few suggestions:
- Instead of using two img tags, I suggest using a picture element, which is incredibly useful in cases where you have two (or more) different images to switch between.
- In the future I would avoid setting express heights, widths, paddings, etc. (basically most things to do with sizing and positioning) on the
body, and instead use another container/wrapper div. It works ok for some of these smaller projects, but you will run into trouble on larger ones. You could probably take the width, height, and paddings and set them on your main container instead without issue. - It's also better, generally, for content to determine the height of an element rather than setting a specific px height. In fact if you remove the heights from your body, it looks like your solution doesn't change at all, so you could probably delete them. 👍You'll get into all of that when you get into responsive design. 😄 Speaking of, the resource page has some great links to learn Flexbox and Grid.
Again, good job on this solution and happy coding!
Marked as helpful
@guilhermerera
Posted
@brasspetals Thank you so much for your feedback and comments, Anna! It was really helpful and made me learn some new things. I had one of those WOW MOMENTS with the picture element. I bet it is pretty basic, but it was a huge step here.
The body/maincontent tip was pretty good. I'll try to look at my smaller projects as a part of a large one, so I can work my code organization/structure right from the beginning to avoid troubles with bad habbits in the future.
I updated the code with your tips and it look and feel way better now. Thanks again!
@brasspetals
Posted
@guilhermerera When I found out about the picture element a handful of months ago, it blew my mind too! How had I gone so long without knowing about this? Something similar you may find useful in future projects is srcset. It's great for situations when you have copies the same image (same dimensions/ratio) but in different resolutions (ex: smaller version for mobile). Here's more about srcset and responsive images on MDN.
Glad my suggestions helped! Looking forward to your next solution! 😄
@guilhermerera
Posted
@brasspetals I'll take a good look at srcset too, thanks a lot!
