News homepage with Flexbox and CSS grid

Community feedback

• 

  P

  visualdennis•8,375

  @visualdennissPosted over 1 year ago

  Hi there,

  Your solution looks nice overall, however there are few issues. Firstly i'd suggest avoid giving fixed height or max-heights for elements/containers that contain text elements. It will cause accessibility issue, when user changes their default font-size, text will overflow and break the layout. In this case you have given to the body a fixed height: 100vh. But when i see your project on my notebook, top part of the page is simply cut off and nav is not appearing. Resulting in data loss, which you never ever want to happen. So instead you can use min-height: 100vh; so it will be at least 100vh but never constraint it to be more than that if necessary.

  Hope you find this feedback helpful

  Marked as helpful

  1

  ---

  

  hassan•390

  @HassanMak29Posted over 1 year ago

  @visualdenniss thank you very much, that was a critical remark! I fixed it, and I will use min-height in the future 👍

  1