html css grid flexbox

Community feedback

• 

  Thiago Souza•80

  @thiagorasouzaPosted over 3 years ago

  Hey Izabela Górska!

  Good work! Intuitive HTML markup and good code structure.

  I have a few comments to make:

  • Using grid-template-areas on div.comments > div would make that part of your code more readable;
  • When applying align-self to .name elements, it's better to use "end" instead of "flex-end", since the container is a CSS Grid in this case, not a Flexbox;
  • On the desktop version of div.comments, the rule grid-template-areas: ". . ."; (dot dot dot) is not necessary. Since you are not creating any references/template and grid-template-rows and grid-template-columns are already set, there is really no need.
  • Your .comments are overflowing a little bit on smaller window widths, probably because you have set a fixed height for them. Instead you could set height: auto; and align-self to "start".
  • The background images are not reaching the bottom end of the window. That's tricky to solve, but doable. The body element is currently only as tall as the content and you need it to be as tall as the window viewport. To solve that, you can set body's min-height to 100vh. Observe now that the page is overflowing, vertical scrollbars appear because the .container's margins are collapsing. In other words, the .container margins end up outside it's parent element (body). So body is actually 100vh + .container's margin-top + .container's margin-bottom. In your case, the fastest way to solve that is to change your margins declarations to padding on .container. Look for "mdn margin collapsing" to read more on that.

  If my feedback helped you in any way, please consider upvoting it!

  Also, it would be great if you could give me your feedback on my solution 😀

  1

  ---

  

  Izabela Górska•100

  @IzaGorskaPosted over 3 years ago

  Thank you so much! Those comments definitely help me improve this project and the ones I will do in the future.

  0