Shashi Lo• 1,345
@shashilo
Posted
Hey Paul. You did a good job implementing this challenge. Does look like you struggled with the footer and where are my suggestions:
- The design calls for the desktop to have a max-width of 1440px. If you add that container, you won't need to add the additional padding and margin to the left and right of the footer. This should help ease the size of the columns.
- Never duplicate header or footer links. That's like double dipping. It's bad for web accessibility and you have to maintain the links in two places. There's a way to remove Flexbox and make the links stacked on the mobile view with media queries.
- Remove the fixed heights in the
#footer-wrapper. There seems to be a few in here. That will mess up with the box model. - Instead of creating 3 separate
<ul>elements for the 3 columns, make 1. Semantically, 1 makes a lot more sense because it's a group of links not separate groups. Either with Flexbox or Grid. https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout/Mastering_Wrapping_of_Flex_Items
1
