Design comparison
Community feedback
- @AdrianoEscarabotePosted about 2 years ago
Hello ericabao, how are you?
Welcome to the front-end mentor community. I really liked the result of your project, but I have some tips that I think you will like:
1- Document should have one main landmark, you could have put all the content inside the
main
tag click here2- All page content should be contained by landmarks click here
I noticed that the media query is activating very early, to fix this we can do the following:
@media (min-width: 446px) { .container{ width: 320px; } }
The rest is great!!
Hope it helps...👍
Marked as helpful0 - @correlucasPosted about 2 years ago
👾Hi Ericabao, congratulations for your first solution!👋 Welcome to the Frontend Mentor Coding Community!
Great start and great first solution! You’ve done really good work here putting everything together, I’ve some suggestions you can consider applying to your code:
Use
<main>
instead of<div>
to wrap the card container, its better to use<main>
in this case because you’re indicating that’s the main block of content of this page and also because<div>
doesn’t have any meaning, it's just a block element.Think about using relative units as
rem
orem
instead ofpx
to improve your performance by resizing fonts between different screens and devices.Anyhow, if we want a more accessible website, then we should use rem instead of px. REM does not just apply to font size, but to all sizes as well.Use a CSS reset to avoid all the problems you can have with the default CSS setup, removing all margins, making the images easier to work, see the article below where you can copy and paste this css code cheatsheet: https://piccalil.li/blog/a-modern-css-reset/
✌️ I hope this helps you and happy coding!
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