QR code component solution using HTML and CSS

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hi, Soojeong! 👋

  I only see you made the body element as a flex container. However, I didn't see any absolute positioning.

  Anyway, I suggest using min-height instead of height on the body element. Right now on mobile landscape view, the card is getting cut off a little bit. It's because the body element only has a specific height which is 100vh. So, by using min-height it allows the body element to grow if needed and it makes sure that the body element always fill the entire page.

  You've done a great job by changing the flex-direction to position the attribution below the card element. I would not suggest using absolute positioning in this case. It makes the element out of the document flow which can cause issues.

  I would like to give some other feedback.

  • Changing the html or root font size can cause huge accessibility implications for those users with different font size or zoom requirements. Read what an accessibility expert (Grace Snow) has said about it.
  • Alternative text should not start with "Image of" or contain any words that are related to "image". img element is already enough to tell the screen readers that it is an image.
  • I suggest improving the alternative text by telling the user what the QR code is for. So, the better alternative text would be "QR code for frontendmentor.io".
  • I highly recommend reading this article to know how to write better alternative text

  That's it! Hope you find this useful!

  Marked as helpful

  2

  ---

  

  Soojeong•220

  @kongguksuPosted over 2 years ago

  @vanzasetia Thank you for your thorough feedback, Vanza! I initially started out using absolute positioning to center the card horizontally and vertically, but switched to using Flexbox after getting the first feedback. Thank you especially for helping with the min-height part! And I don't think I checked the mobile version for it. I'll definitely try fixing this issue!

  0

  

  Soojeong•220

  @kongguksuPosted over 2 years ago

  @vanzasetia I have a follow-up question about the changing the root font size. I have been using this method of resizing root font size to 62.5% and it's been comfortable because this makes it easy for me to calculate the font sizes in rems since this makes 10px = 1rem which is very intuitive. Reading what you linked me has helped me understand why it's not recommended. But I'm wondering then, would I have to calculate the font sizes(px) to rem by dividing each of them by 16 each time I'm thinking of the font sizes?

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @kongguksu Well, I guess you don't have to calculate everything. 🙂

  For me, I use Sass so I can create a rem function which basically turns any numbers that I input as a pixel into a rem unit.

  /* In Sass */
  body {
    font-size: rem(16);
  }
  
  /* Compiled CSS code */
  body {
    font-size: 1rem;
  }
  

  When I did one of the Frontend Mentor challenges using CSS, if I'm not mistaken, I calculated all the px to rem using a calculator. However, if the result that I got was 1.33333 then in CSS I would write it as 1.3rem.

  So, changing the root font size is convenient only for developers and can cause issues for users. The website is for the users so it's best to prioritize the users. It's just my opinion. 🙂

  Marked as helpful

  1

  

  Soojeong•220

  @kongguksuPosted over 2 years ago

  @vanzasetia Thanks again! I haven't learned Sass yet, but I'm planning to, after I learn JS. I can use your method in the future then. Meanwhile, I think I'll try just calculating pixels into rem 😁

  0
• 

  Claude•290

  @claude1018Posted over 2 years ago

  Nice solution.. For centering. I think it will depend on your purpose.

  And for your concern on why the attribution is stack at the top it is because you use position: absolute on your qrcode-card class. since the qrcode-card class is out of the document flow so the footer/attribution is stack at the top. If you want to fix it you can wrap the qrcode-card in some div tag or a landmark so it can also solve the html acessibility issue.

  Ohhh.... BTW Add a position: relative on your wrapper

  though you can also use flexbox on the wrapper to center things inside it without using any absolute

  Marked as helpful

  1

  ---

  

  Soojeong•220

  @kongguksuPosted over 2 years ago

  @claude1018 Thank you for your feedback! I used Flexbox instead of Absolute Positioning by wrapping my qrcode-card just like you said and it worked well. I was also able to move the attribution to where I wanted it to be :)

  0