QR code component layout using CSS Flex.

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  Hi, Abu! 👋

  Congratulations on finishing this challenge! 🎉

  Here are some suggestions for improvements.

  • The alternative text for the QR can be improved by giving some information about the QR code. In this case, it is going to navigate the users to https://www.frontendmentor.io/ after they scan the QR code. So, the better alternative is something like "QR code to frontendmentor.io".
  • Put the attribution below the card. Change the flex-direction of the flex container (body) to column. Then, remove the display: flex; from the .attribution.
  • I recommend wrapping the card with the main landmark. For the attribution, it should be wrapped with footer because it is not the main content of the page.
  • Never set max-width on the body element. It should always be filling the entire page. You should only limit the width of the elements inside the body element.
  • Remove height from the body element too. The body element should always fill the entire page. Treat it as the main element of the web page.
  • Remove hieght from the .container or the card. Let the elements inside the card control its height of it.
  • Prefer unitless numbers for line-height values to avoid unexpected results. The MDN documentation of line-height explains it well.

  I hope this helps! Happy coding!

  Marked as helpful

  1

  ---

  

  Abu Kesh•20

  @AbuKeshPosted about 2 years ago

  @vanzasetia Thank you, for your suggestions. I will look into this and edit my solution. But about that removing height from the body element, I tried it first but without giving the height to the body that container is not placing in the middle of the body vertically. So, thats why I have given the height to the body.

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  @AbuKesh You are welcome! 👍

  You can set min-height: 100vh instead. This way, if the card needs more than 100vh, then the body element can grow to fit the content.

  0
• 

  Melvin Aguilar 🧑🏻‍💻•61,140

  @MelvinAguilarPosted about 2 years ago

  Hi @AbuKesh 👋, good job completing this challenge! 🎉

  I have some suggestions you might consider to improve your code:

  • Use the <main> tag to wrap all the main content in your solution instead of using <div class="container"> to improve the accessibility of the website.

  • Use <footer> instead of <div class="attribution">. The <footer> element contains authorship information.

  • To make alternative texts more worthwhile, add descriptive text to the alt attribute of the QR image to explain what the QR image does. Upon scanning the QR code, you will be redirected to the frontendmentor.io website, so an example of alternative text would be "QR code to frontendmentor.io". You can read more about alternative text here.

  • Instead of using pixels in font size, use relative units of measure like rem or em. The font size in absolute length units (px) does not allow users with limited vision to change the text size in some browsers. Reference.

  • Use min-height: 100vh to the container selector instead of height. This property lets you set a height and let the element grow even more if necessary.

  • Use flex-direction: column; to body selector to place the footer below the component

  Above all, the project is done well👏. I hope those tips will help you! 👍

  Good job, and happy coding! 😁

  0

  ---

  

  Abu Kesh•20

  @AbuKeshPosted about 2 years ago

  @MelvinAguilar Thank you for your feedback.

  0