QR CODE CHALLENGE USING MARGIN, PADDING

Please log in to post a comment
Log in with GitHubCommunity feedback
- @solvman
Very well done! 🎊🎉🚀
I have a few suggestions for you:
-
⭐️ I suggest the use of semantic elements instead of just
<div>
and<span>
(both are non-semantic). 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. -
⭐️ 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 */ }
- ⭐️ You set the body as a Flex container but then try to play with
position: absolute
to center the card. Though it does the job, it looks way more complicated than it needs to be, adding clipping issues on smaller heights. To place the card in the middle of the screen, you could use Flex or Grid. I prefer the Grid since it is only a line shorter:
body { height: max(500px, 100vh); display: grid; place-content: center; }
-
⭐️ Consider using REM units for margin, padding, and font size.
-
⭐️ Consider the use of custom global variables. 👍
Otherwise, very well done!🎊 Keep it up!👏 I hope you find my comments useful 🫶
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