Design comparison
Solution retrospective
Recap on Html and CSS used goggle to re-visit of the html and css code example to refresh my knowledge.
Community feedback
- @Helter5Posted 4 months ago
Site is good and its also responsive. Code is readable and well-structured. Solution has structurally have same logic.
Marked as helpful0 - @gmagnenatPosted 3 months ago
Hi, congrats on completing this challenge ! I see you completed several challenge but they all have some common issues on fundamentals that you should work on before moving to more complex layouts. I hope you'll find something useful in these comments to help you improve your coding skills.
Does the solution include semantic HTML?
- You are missing a <main> landmark to indicate the main content of the web page.
- the alt attribute of the qr code should describe that image for example saying "QR Code to frontendmentor.io" so someone who cannot see that image know what it is about.
- the title should be a heading (h1, h2, h3..), not a <p>.
Is it accessible, and what improvements could be made?
- You can improve the accessibility for some users who are increasing the browser default font-size to see the text larger. By setting these value in pixels you don't allow the user to update this and force on one single value.
- Avoid using large margin to position your content. Its an indicator that something isn't right. To center your card on the page you can use flexbox.
body { display: flex; flex-direction: column; justify-content: center; align-items: center; min-height: 100svh; }
- the min-height on the body allows flexbox to position the content in the center. Using min-height is important as the body height can also grow if the font-size is changed for example.
- use relative units for your font-sizes such as
rem
. Use relative units for your max-width inrem
as well. A container with text, need to be flexible and able to scale according to the user preference. If you set fixed value you can have issues with content overflowing.
Does the layout look good on a range of screen sizes?
- It looks quite small compared to the design you can inspect the values in the figma file for this project.
Is the code well-structured, readable, and reusable?
- You need to add a modern css reset at the start of your css file. It will help you deal with weird differences between browser by reseting these things you don't need to focus on. It's a good habit to take early on.
- when working on your css try to organise it top to bottom based on the design. In your css I see styles for the image (first element) at the very bottom of your stylesheet for example.
I hope this helps you refactor this solution and your other ones. Understanding and fixing most of these issues will be very beneficial to avoid complex layout problems later.
Happy coding !
1@LeowWeiLeePosted 3 months ago@gmagnenat
Please help review if I have gotten your comment correctly.
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