“Mobile-first solution using CSS Flexbox and javascript

Please log in to post a comment
Log in with GitHubCommunity feedback
- @grace-snow
Hello
This looks pretty good but there are some problems
- Height 100vh is causing the body background to stop before the content on mobile landscape. Use min height instead
- Never ever use width 100vw. All that does is cause horizontal scroll in some views because whenever there is a scrollbar present, you are effectively telling the browser you want the body to be "100% wide PLUS the width of the scrollbar). Viewport units don't account for things like that. You don't need a width on the wrapper at all. It's full width anyway
- It's not good practice to load 3 separate css files, especially on a tiny challenge like this. Combine into one for good performance
- Never style on IDs. It's not what they're for and is messing with the css specificity tree in your styles. Similarly you are nesting selectors a lot here. That will become impossible to manage on bigger projects. Use single class selectors as much as possible and try to keep css specificity low / flat
- There's no need to give main a min width (or a min height actually)
- Font size must NEVER be in px, even in your css reset. This is extremely important. That has immediately made this inaccessible to some users
- In html if decorative svgs are inline they must have
aria-hidden="true" focusable="false"
. In fact its good t I do that on any inline svgs including meaningful ones and label them with separate text if they are meaningful - Remove
aria-label="submit button"
. That is already a button and already has text inside saying submit. The golden rule of aria is to only use it when needed. - If you do want to add some aria an important place to add it would be
aria-live="polite"
on an additional wrapping element eg div around the thank you content. This would tell the accessibility api to effectively watch for changed. When that thank you content gets displayed it will them be immediately read aloud
Nice job on the javascript. It's optional but if you want to use a well established code standard there, I recommend using javascript specific selectors in your html. For example, anything you need to grab in JS, use a
js-
prefixed class name. That way it's really obvious to devs what classes are being used for styling and what classes are being used for javascript.
Join 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