Design comparison
SolutionDesign
Community feedback
- @thomashertogPosted about 1 month ago
This feels not completely finished to me. You're on your way there though. Overall, this solution is looking good! You've used accessible/semantic HTML where possible
A few things I've noticed
Design
- you're not using the full height of the window if it is bigger than your content
- there's no whitespace on the left your content when you look at it in the desktop view but slowly going narrower
- I saw an error image in your code but couldn't get to visualize it by using the page
HTML
- Your code is bloated with
<div>
wrapper elements where I feel things could be much simpler, you don't need a wrapping element for your form items (e.g. input field and submit button) - You include both hero images (desktop and mobile) only to hide them with your CSS. Beware that this causes both images to be downloaded (and using data), you may want to look into the
<picture>
element to resolve this - You have an
<h1>
on the page (which is required for accessibility reasons), however I feel We're coming soon is not really helpful as a header. I'd suggest making the name of the company an<h1>
(and hide it accessibly)
CSS
- There's some repeated statements here and there in your media queries. Be aware that if you have them in your normal CSS, they cascade over to your media queries (e.g.
flex-direction: column
on your body selector and again for your body selector inside a media query) - It feels a little awkward that you specified 3 rows for a grid but are using 4 rows, maybe you want to explicitly set their heights as well or (I'd do it this way I think) use sizing of the elements and let those determine the height of the rows which will be auto-sized if you eliminate the
grid-template-rows
altogether - I feel there's a lack of structure in your CSS code, there is no grouping/ordering of properties that makes sense, selectors are thrown around and are varying a lot in specificity (read up on specificity graph to see if you can make some adjustments here)
- A lot of sizings are fixed (using absolute units like
px
, you could benefit from converting those to relative units like%
) - There's a lot of flexbox stuff going on where I feel you have done some overkill (probably as well because of all the wrappers)
Marked as helpful0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord