Design comparison
Community feedback
- @faha1999Posted about 2 years ago
Hello, Stanley Congratulations on finishing this project. It's lovely and great on the whole! Just a little tip:
- You might want to use semantic tags like the
<main>
to wrap your code, instead ofdiv
. like
<main class="container"></main> <footer class="attribution"></footer>
This would help improve accessibility.
- add the below styles to the
body
. It will center everything
body { justify-content: center; align-items: center; display: flex; min-height: 100vh; flex-direction: column; }
- Add the below styles to the
.container
.container{ max-width: 40rem; /* margin: 7rem 15rem 9rem 15rem; */ /* border: 1px solid hsl(30, 38%, 92%); */ }
Remove all margin
-
Fixed the media issue.
-
Instead of using
px
, use relative units likerem or em
to get better performance when the information on your page needs to be resized for multiple screens and devices.REM
andEM
apply to all sizes, not justfont-size
. You can code your entire page inpx
and then, at the very end, use the VsCode pluginpx to rem
to perform the automatic conversion px to rem
I hope it will work. Happy coding.
Marked as helpful0@dadsec-devPosted about 2 years ago@faha1999 Ahh! Thanks alot man. Your suggestions where super usefule, i've updated the code but the media-queries still isn't showing the image... Any help there will be very much appreciated
0@faha1999Posted about 2 years ago@dadsec-dev I already made a pull request to the repo. Check it. Your media-queries & other accessibility issues are solved.
0 - You might want to use semantic tags like the
- @dadsec-devPosted about 2 years ago
I encountered an issue with the mediaqueries, could'nt work on the mobile properly. i'll update later
0
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