HTML5 and CSS3 Section

Community feedback

• 

  Anna Leigh•5,135

  @brasspetalsPosted over 3 years ago

  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

  3

  ---

  

  Guilherme Ferreira•65

  @guilhermereraPosted over 3 years ago

  @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!

  0

  

  Anna Leigh•5,135

  @brasspetalsPosted over 3 years ago

  @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! 😄

  1

  

  Guilherme Ferreira•65

  @guilhermereraPosted over 3 years ago

  @brasspetals I'll take a good look at srcset too, thanks a lot!

  0
• 

  P

  Patrick•14,265

  @palgrammingPosted over 3 years ago

  Looks good but you might want to try to build a middle layout to use the space more effectively between the mobile and desktop layout. Using the desktop sized elements but in the mobile layout

  1

  ---

  

  Guilherme Ferreira•65

  @guilhermereraPosted over 3 years ago

  @palgramming oh, nice sugestion, thanks! I'll try to build that. :D

  0
• 

  Lorenzo•45

  @LorenzoChioPosted over 3 years ago

  Don't wanna read your code cause I'm also doing this one but congratulations! It looks great.

  1

  ---

  

  Guilherme Ferreira•65

  @guilhermereraPosted over 3 years ago

  @LorenzoChio thanks! looking forward to check your solution too!

  0

  

  Lorenzo•45

  @LorenzoChioPosted over 3 years ago

  @guilhermerera Thanks, will post it here when it's up!

  0