Design comparison
Community feedback
- @gmagnenatPosted 4 months ago
Hi, congrats on completing the challenge !
I noticed a few things that you can improve in your solution and your future challenges. I hope you will find the comments useful.
Does the solution include semantic HTML?
- Good use of the main and footer landmark !
- You can add a more detailed alt for the QR code image. For example here it could indicate where this QR code is leading.
Is it accessible, and what improvements could be made?
- Currently this solution has accessibility issues for users who update their browser font size.
- Always use relative units for fonts. Why font-size must NEVER be in pixels
- use max-width in
rem
instead of a fixed width in pixels. It will allow the layout to scale correctly in case of a larger font. - don't set height on : your card, your image, the body. They don't serve anything and don't allow the layout to scale.
- use a modern css reset at the beginning of your stylesheet. Check Josh Comeau or Andy Bell, both are well known and great. It will fix the common browser default styling. It will also add a max-width and display block to images.
- width of the body is already 100% you can remove it as well.
Does the layout look good on a range of screen sizes?
- yes
Is the code well-structured, readable, and reusable?
- I'm personnally not fond of the camecase for class names. I don't recall seing this used much for css classes and honestly I find it diffigult to read. It's great for javaScript classes of course...
Does the solution differ considerably from the design?
- No. It is close to the design and respect the design intentions.
I hope you find something useful in these comments. Don't hesitate to reach out on discord if you have any questions on these points.
Happy coding !
Marked as helpful0@defPhisyPosted 4 months agoThanks @gmagnenat, that helps a lot and i am happy to get so detailed feedback. You are right the camel case is not optimal, i´ll change that. what i am most curious about is the "modern css reset" :)
0@defPhisyPosted 4 months ago@gmagnenat Thanks for the modern css-reset advice. finished my now my css-reset.css on my second challenge. Blog preview card
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