Responsive QR code component

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hello, KurtJF! 👋

  Congratulations on completing your first Frontend Mentor challenge! 🎉

  Regarding your questions:

  • It's possible to finish this challenge without using any div elements. I suggest removing the div with the class of .card. Also, it's not a necessary thing to wrap the text elements (h1 and p) with a div. I suggest making those elements as the child elements of the main element. For the attribution, you can swap the div with a footer instead.
  • I think you have done a great job on using CSS custom properties. But, it's hard to tell how well it is since the project is a small project though. 😅

  Some suggestions from me.

  • The alternative text for the QR code can be improved. Try to tell the users about the QR code. By the way, have you tried to scan the QR code?
  • I recommend adding width and height attributes to each image element. This way, the browser knows how much space the image requires before it can be fully loaded. As a result, it would optimize CLS (Cumulative Layout Shift). As a side note, it is not the same as width and height CSS properties. So, make sure you learn how to use them.

  That's it! I hope you find this useful! 😊

  Marked as helpful

  1

  ---

  

  P

  KurtJF•150

  @KurtJFPosted over 2 years ago

  @vanzasetia Thank you for your detailed response regarding my questions! It really helped a lot, Also I have updated my solution.

  If you don't mind me asking, How can I tell if the div is not needed?

  Again thank you very much for the comments and I will keep them in mind when tackling the other challenges. 👍

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @KurtJF Happy to hear that was helpful! 😊

  In this case, you don't need any div because you can use the main element as the container of the elements inside it. Also, the attribution should be a footer to fix the issue that has been reported.

  In general, you want to keep the HTML markup as simple as possible. So, I recommend trying to write as minimum HTML as possible. After that, if it needs div to wrap the elements in order to create two-column layouts then use a div.

  The point is to use div when it is needed. 🙂

  By the way, good job with the update! The HTML markup looking good to me. 👏

  Marked as helpful

  0
• 

  Lucas 👾•104,400

  @correlucasPosted about 2 years ago

  👾Hello again Kurt, congratulations for your new solution!

  Great great great solution you've here, I liked a lots that you html structure is super clean this is amazing Kurt. If you want to clean even more you code you can remove all the classes and use the direct selector for each element to manipulate the CSS, for example select everything with main, img, h1 and p.

  All you need to make this container responsive is to replace width: 320px; with max-width: 320px;

  .card-container {
      border-radius: 22px;
      margin: auto;
      background-color: var(--white);
      max-width: 320px;
      padding: 16px;
  }
  

  👋 I hope this helps you and happy coding!

  0