QR Code Component Challenge

Solution retrospective
Thank you for any feedback.
Please log in to post a comment
Log in with GitHubCommunity feedback
- @solvman
Very well done! 🎊🎉🚀
I have a few suggestions for you:
-
⭐️ Great job using semantic element
<main>
. Semantic elements significantly improve the SEO and accessibility of your project. First, the<main>
landmark element represents the primary content of the document and expands on the central topic of the document. You should wrap your content in<main>.
Such widgets as cards are more suited to be constructed with the<article>
element, which encapsulates reusable, self-contained content. However, the<seciton>
element is unnecessary since it has no semantic significance. -
⭐️ Titles and headings are usually denoted by
<h1>
,<h2>
,<h3>
, and so on. Do not skip levels of headings. Regular text is generally encapsulated by<p>.
A card-like widget's most appropriate heading level is likely<h2>
.
With that being said, I would redo your code as so:
<body> <main id="container"> <h1 class="visually-hidden">Frontend Mentor project submission</h1> <article class="card"> <img src="./images/image-qr-code.png" alt="QR Code"> <h2>Improve your front-end skills by building projects</h3> <p>Scan the QR code to visit Frontend Mentor and take your coding skills to the next level</p> </article> </main> <footer class="attribution"> ... attribution goes here </footer> </body>
As mentioned above, the
<h2>
heading is the most appropriate for the card-like widget. To avoid breaking hierarchy heading rules, I added an invisible<h1>
heading to announce "Frontend mentor project submission" to accessibility users. Visually hidden class (it is also calledsr-only
which is "screen reader only") for the<h1>
:.visually-hidden { position: absolute; width: 1px; height: 1px; padding: 0; margin: -1px; overflow: hidden; clip: rect(0, 0, 0, 0); white-space: nowrap; border: 0; }
Learn more about semantic HTML elements here
Please remember that block-level elements stack one on top of the other. The only element that is not block level within the card is
<img>,
which could be "converted" to block level through a simple reset, which should be used almost on every project anyways:img { display: block; max-width: 100%; /* ensures images does not overflow the container */ }
-
⭐️ The reset above will ensure the images stretch to fit the containing element. There is no need to set the image size; doing so makes the image not responsive to viewport resizing.
-
⭐️ Great job using Grid to place your card in the middle of the screen. 👍
-
⭐️ Be consistent using REM units for margin, padding, and font size. 👍
-
⭐️ Great use of custom global variables. 👍
Otherwise, very well done!🎊 Keep it up!👏 I hope you find my comments useful 🫶
Marked as helpful -
- @cchivers
Looks really good, I would drop the
font-weight
of the heading. Other than that it looks amazing. Keep up the great work!Marked as helpful
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