Submitted over 2 years ago

QR Code Component

Kenisa•90

@KenisaRenee

Preview site View code

This is a solution for...QR code component

HTML
CSS

1newbie

Design comparison

SolutionDesign

Solution retrospective

I am still very new to all things web development. I don't know enough to create good questions, but any and all feedback is appreciated! Also if there is a way my code could have been written cleaner, please let me know.

Community feedback

Elaine•11,400
@elaineleungPosted over 2 years ago
Hi Kenisa, well done on completing your first challenge on Frontend Mentor! I think you did an excellent job here with your solution. In terms of feedback:

First off, you got a box-sizing rule as a reset, so good job! I would also add at least a rule underneath for images since you're using one in this challenge. That way, your images will mostly work fine when you add them in.

img, picture, video, canvas, svg { display: block; max-width: 100%; }

I see that you got some fixed widths on your card and image, which can be easy to use when starting out, but with more complicated layouts, you'll almost always want to use either responsive widths (e.g., max-width) or just let the contents or parent container determine the size, instead of needing to set one.

Here is something you can try:

If you added the rule I mentioned above regarding images, you can go ahead and remove all the widths, margins, and heights for the card and image (including the elements inside), and then set a max-width:300px on .card with padding: 1rem. Next, add a new div container under .image, and put both the h1 and p inside this new div container; give the container a padding: 1.5rem 0rem 2rem, and also a margin-bottom: 1rem for h1. With this, you only need one max-width, and you can just rely on the padding and contents to determine the height and width.

Anyway, you're really doing great, so keep working on these challenges, and happy coding!
Marked as helpful
2
Kenisa•90
@KenisaReneePosted over 2 years ago
@elaineleung Wow, Elaine! Thanks so much for the awesome feedback and encouragement. I will implement your suggestions to the best of my ability!
1

Lucas 👾•104,340

@correlucasPosted about 2 years ago

👾Hello Kenisa, Congratulations on completing this challenge!

The html structure is fine and works, but you can reduce at least 20% of your code 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>.

YOUR CURRENT CODE:

<body cz-shortcut-listen="true">
  <main class="container">
    <div class="card">
      <img src="/images/image-qr-code.png" alt="QR code">
      <div class="description">
        <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>
      </div>
    </div>
  <div class="attribution">
    Challenge by <a href="https://www.frontendmentor.io?ref=challenge" target="_blank">Frontend Mentor</a>. 
    Coded by <a href="https://www.frontendmentor.io/profile/KenisaRenee" target="_blank">KenisaRenee</a>.
  </div>
</main>
</body>

THE CODE CLEANED:

<body>
<main>
<img src="./images/image-qr-code.png" alt="Qr Code Image" >
 <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>

To reduce the CSS you can use the direct selector for each element instead of using class this way you have a code even more cleaner, for example, you can select everything using the direct selector for (img, h1, and p, main).

✌️ I hope this helps you and happy coding!

Marked as helpful

Please log in to post a comment

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