Design comparison
Community feedback
- @Yehan20Posted over 1 year ago
Hello there , congratulations on completing the solution it looks awesome , it would be good practice to name the alt tag in the image , with a meaningful name, also i noticed that you have installed the Google font in your CSS and HTML both , that's not needed you can remove one of them , I'd recommend removing the CSS import. Next its considered good practice to add a universal selector in the top of your style sheet and set the box-sizing to border box which tells your the browser to account for any border and padding in the values you specify for an element's width and height. You can try to increase the width of the card as it kind of looks less long than in the design , i hope this helped good luck in other challenges .
Marked as helpful0 - @shakhboz-shukhratPosted over 1 year ago
Hello there👋! Congratulations on completing this challenge!
There are no major issues in the provided code. However, there are a few minor points that can be improved:
The "margin: 50px 0;" on the .qr-content selector in the main CSS could be more specific by adding margin-top and margin-bottom separately to avoid unintended margin collapsing effects.
The "border-radius" values should be in pixels, not in percentages.
The "font-size:15px;" on the .qr-body selector and .qr-content h3 and p selectors are repeated and could be defined in a single place to avoid code duplication.
Here is the updated code with these minor improvements:
@import url('https://fonts.googleapis.com/css2?family=Outfit:wght@500;700&display=swap'); :root { --body-font-size: 15px; } .qr-body { margin: 0; padding: 0; background-color: hsl(211, 100%, 94%); font-size: var(--body-font-size); } .attribution { font-size: 11px; text-align: center; } .attribution a { color: hsl(228, 45%, 44%); } .qr-content { background: #fff; width: 20%; padding: 1%; border-radius: 5px; margin-top: 50px; margin-bottom: 50px; } .qr-content img { width: 100%; margin: 1% 0; border-radius: 3px; } .qr-content h3 { font-size: 1.2rem; font-weight: 700; font-family: 'Outfit', sans-serif; } .qr-content p { font-size: 1.2rem; font-weight: 500; color: hsl(220, 15%, 55%); font-family: 'Outfit', sans-serif; } @media only screen and (max-width: 375px) { .qr-content { width: 100%; padding: 3%; margin-top: 30px; margin-bottom: 30px; } .qr-content img { width: 90%; margin: 3% 5%; border-radius: 3px; } .qr-content h3 { margin: 2%; } .qr-content p { margin: 5%; } }
Anyway, your solution is great. Hope you will find this helpful. 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