Design comparison
Solution retrospective
π£ My 2nd Newbie(free) project: Please help to advise. Thank you for any feedback. π» βοΈ
Community feedback
- @VCaramesPosted about 2 years ago
Hey @MissSiriluck, adding to what was said above/below:
-
The Alt Tag description for the QR image needs to be improved upon. Its needs to tell screen reader users what it is and where it will take them to when they scan it.
-
To make your image fully responsive delete the
widthand add the following make it part of your CSS Reset (so your images will always be responsive In future projects.)
.img-qr-code { max-inline-size: 100%; block-size: auto; }Happy Coding!
Marked as helpful1 -
- @correlucasPosted about 2 years ago
πΎHi @MissSiriluck, congratulations on your solution!π Welcome to the Frontend Mentor Coding Community!
Great solution and a great start! From what I saw youβre on the right track. Iβve few suggestions for you that you can consider adding to your code:
1.Reduce your code by removing unnecessary elements. The HTML structure is working but you can reduce at least 20% of your code by cleaning the unnecessary elements, you start cleaning it by removing some unnecessary
<div>. For this solution you wrap everything inside a single block of content using<div>or<main>(better option for accessibility) and put inside the whole content<img>/<h1>and<p>.<body> <main> <img src="./images/image-qr-code.png" alt="QR Code Frontend Mentor" > <h1>Improve your front-end skills by building projects</h1> <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p> </main> </body>2.Use units as
remoreminstead ofpxto improve your performance by resizing fonts between different screens and devices.To save your time you can code your whole page using
pxand then in the end use a VsCode plugin called px to rem here's the link β https://marketplace.visualstudio.com/items?itemName=sainoba.px-to-rem to do the automatic conversion or use this website https://pixelsconverter.com/px-to-remHere's my solution for this challenge if you wants to see how I build it: https://www.frontendmentor.io/solutions/qr-code-component-vanilla-cs-js-darklight-mode-nS2aOYYsJR
βοΈ I hope this helps you and happy coding!
Marked as helpful1@MissSiriluckPosted about 2 years agoβ€οΈ Thank you for your guidance. @correlucas. I'll keep your advice to keep improving my coding skill.
0 - @KristinaRadosavljevicPosted about 2 years ago
Hey, great job on this project, your solution looks really good :)
There are just two slight adjustments that you might want to consider making:
- You don't have to use so many
<br>elements in your HTML - the text will break naturally once the proper layout is set up. - I guess the centering technique you used with
position: absolute;andtranslate()works in most cases, but it fails a bit when you shrink the screen vertically (which would happen if you view your project on mobile in landscape mode, for example). So, I and many other people here opted for making thebodya flex container and centering the<main>element by usingalign-itemsandjustify-contentproperties. You would also need to setmin-height: 100vh;on the body for this to totally work.
I hope this helps and once again, good work! :)
Marked as helpful1@MissSiriluckPosted about 2 years ago@KristinaRadosavljevic Thanks a lot for your advice, Kristina . I try to do other way to coding. Your comment is helpful. I will take it to improve this solution again.
1 - You don't have to use so many
Please log in to post a comment
Log in with GitHub
Join 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