@GregLyons
Posted
Nice work! It looks you have a pretty good grasp of semantic HTML, as well as Flexbox. I have a couple notes on your CSS:
Are you deliberately using box-sizing: content-box; (at the top of your CSS)? I believe that's the default behavior, so you shouldn't need to declare it. On the other hand, a lot of developers (myself included) prefer box-sizing: border-box;. For a lot of people, it makes reasoning about widths and heights easier, as margins and padding are factored into any specified width/height, as opposed to applied in addition to (and hence changing) the specified width/height. If you don't know about that, you may want to try it out. If you prefer content-box instead, though, then by all means keep using it.
The other thing I'll mention is that your height: 100vh; rule on the <body> suffices to center your content vertically, but just in case for your future apps/web pages, min-height: 100vh; is more robust. In the case of your solution it wouldn't change anything, but it would allow the height of the <body> to increase to contain additional content.
In general, you don't want to explicitly set the height of an element (unless, for example, you're trying to get some exact positioning), as different users' browsers have different font-size preferences. So even if your content fits into your element with an explicit height, these different font-sizes might cause the text to overflow your element on the user-end. Using min-height allows you to set your desired height, while still allowing for overflow (e.g. the background of the <div> would grow to contain the overflowing content in my above example). You may already know this and are just using height: 100vh; since in this challenge it won't cause any ill-effects, but I wanted to point this out just in case.
Hope this helps. If so, feel free to Mark this comment as Helpful. Either way, good luck on your future projects!
