Design comparison
Community feedback
- @hitmorecodePosted over 1 year ago
Congratulations well done. After taking a look at your markup, there is one div to many. You have one open div and two closing div's. The img, h1 and p tags are on them same level
### Original markup <body> <div class="card"> <img class="img" src="images/image-qr-code.png" alt="qr-code" /> <h1>Improve your front-end skills by building projects</h1> <p id="grey">Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p> </div> </div> </body>
### modified markup <body> <div class="card"> <img class="img" src="images/image-qr-code.png" alt="qr-code" /> <h1>Improve your front-end skills by building projects</h1> <p id="grey">Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p> </div> </body>
You can also do some changes to your CSS. Instead of adding the font family to each tag, you can place it in the body and it will be applicable for the entire page
### Original CSS body { background-color: hsl(212, 45%, 89%); } h1 { color: var(--Dark-blue); font-family: 'Outfit', sans-serif; font-weight: 700; text-align: center; font-size: 20px; } p { font-size: 15px; font-family: 'Outfit', sans-serif; font-weight: 400; text-align: center; } #grey { color: hsl(220, 15%, 55%); } /* Mobile styles */ @media only screen and (max-width: 375px) { .card { box-shadow: 0 4px 8px 0 rgba(147, 147, 147, 0.2); transition: 0.3s; border-radius: 15px; background-color: white; max-width: 300px; margin: 0 auto; padding: 1rem; margin-top: 170px; padding-bottom: 25px; } .img { border-radius: 15px; width: 100%; } } /* Desktop styles */ @media only screen and (min-width: 376px) { .card { box-shadow: 0 4px 8px 0 rgba(147, 147, 147, 0.2); transition: 0.3s; border-radius: 15px; background-color: white; max-width: 300px; margin: 0 auto; padding: 1rem; margin-top: 170px; padding-bottom: 25px; } .img { border-radius: 15px; width: 100%; } }
### Modified CSS body { background-color: hsl(212, 45%, 89%); font-family: 'Outfit', sans-serif; } h1 { color: var(--Dark-blue); font-weight: 700; text-align: center; font-size: 20px; } p { font-size: 15px; font-weight: 400; text-align: center; color: hsl(220, 15%, 55%); } .card { box-shadow: 0 4px 8px 0 rgba(147, 147, 147, 0.2); transition: 0.3s; border-radius: 15px; background-color: white; max-width: 300px; margin: 0 auto; padding: 1rem; margin-top: 170px; padding-bottom: 25px; } .img { border-radius: 15px; width: 100%; } /* Mobile styles */ @media only screen and (max-width: 375px) { ### inside media queries only add things that you want to change. For example if you want to change the width of the card when the screen width is 375px. .card { max-width: 250px; ### you don't have to write everything down again. } } /* Desktop styles */ @media only screen and (min-width: 376px) { ### inside media queries only add things that you want to change. }
If you have any questions, feel free to ask.
Marked as helpful0 - @brinereef1Posted over 1 year ago
Congrats on complete your first project.
I have one suggestion, your design is breaking on 375px width device
So you can try both min and max media screen to 375px.
@media only screen and (max-width: 375px) @media only screen and (min-width: 375px)
Apart from that your solution is great 👍
Good luck
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