Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Solution using css Flexbox and custom properties

Daniel 130

@tenczowy

Desktop design screenshot for the QR code component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


I would like to get some feedback on clarity and readability of my code. If you had to take over the project after me, how can I improve clarity of my code to make life easier for the following people working on my project?

Community feedback

P
solvman 1,670

@solvman

Posted

Very well done! 🎊🎉🚀

I have a few suggestions for you:

  • ⭐️ First, the <main> 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="wrapper">
    <h1 class="visually-hidden">Frontend Mentor project submission</h1>
    <article class="qr-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 (something it is called sr-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

  • ⭐️ You really do not need an <div class="text-area">. You can access all the same elements using the .qr-card class.

  • ⭐️ Great job using REM units for margin, padding, and font size!

Otherwise, very well done!🎊 Keep it up!👏 I hope you find my comments useful 🫶

Marked as helpful

0

Daniel 130

@tenczowy

Posted

@solvman thank you very much for your in depth answer. I will do my best to apply your suggestions in my future projects!

1

Please log in to post a comment

Log in with GitHub
Discord logo

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