Design comparison
Community feedback
- @Islandstone89Posted about 1 year ago
Hi there, here is my feedback, hope it helps.
HTML:
- The alt text need to describe the content (QR code) and where it leads (frontendmentor.io).
CSS:
-
It's best practice to not import fonts, but link them in the head of the HTML.
-
Font-size must not be in px. Use rem instead.
-
Remove vw width.
-
It's common to center the card horizontally and vertically. You can include the footer as well, by doing this on the body:
display: flex; flex-direction: column; justify-content: center; align-items: center; min-height: 100vh
-
Remove margin-top. Change margin:auto to something like 1rem, to prevent the card from reaching the viewport edge.
-
Put text-align: center on the body, then you dont need to put it any other places because of inheritance.
-
max-width should be in rem.
-
The media query should also be in rem. In it, you dont need to re-declare values that are already set, only the ones you need to override. In this project you actually don't need a media query at all.
Marked as helpful2 - @VarchronosPosted about 1 year ago
- The deployed link is not available.
- I will not talk about the code since I don't see any...
- I will recommend one thing, deploying project is much more easy in vercel than in github pages, so you can use vercel to deploy your work
0@nemoandersonPosted about 1 year ago@Varchronos I update the file. Thanks for letting me know.
1
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