QR code - component

Community feedback

• 

  Karthi Krishnamurthi•110

  @Karthi13krishnaPosted about 2 years ago

  You have done a good job! I think you can make a few changes to improve your website.

  1. I have observed that you are trying to center the card component using padding. I would recommend using Flexbox, Absolute Positioning, or Transform and Translate. You can refer this link.
  2. Use heading elements for headings.
  3. Use padding to add space around an element's content, and use margin between different elements to separate them.

  PS: I'm new to HTML and CSS, and there might be a better approach. I hope this helps.

  Marked as helpful

  0

  ---

  

  Roland Jae V. Patalita•20

  @Rjae07Posted about 2 years ago

  @Karthi13krishna

  Hello Sir,

  Thank you so much for your feedback, I will use this for my next challenge.

  God bless always Sir.

  Thanks you a lot again!.

  1
• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  Hi, Roland! 👋

  Congratulations on completing your first Frontend Mentor challenge! 🎉

  Before diving into the margin and padding, I would recommend setting all the elements box sizing to border-box. This way, the element will not add more width when it gets padding.

  The padding will only be used to prevent the elements touching the edges of the card and on the body element to prevent the elements inside the body element touch the edges of the browser.

  For margin, you can use it to give some vertical whitespace between the elements inside the card.

  There are things that need to be fixed.

  • There is no need for using div to complete this challenge. I will show you how in some of the next points.
  • There's no need for position: relative. I would recommend only using it if you want to use top, right, bottom, and left properties or if there is an element inside it that needs absolute positioning.
  • Use main element as the card element. Then, use flexbox to make the card in the middle of the page by making the body element as the flex container of the main element.
  • For the .attribution, swap the div with footer.
  • I would recommend making the first paragraph as h1 or h2. It is important for a page to have a heading (which is like a title of a document).
  • I suggest improving the alternative text of the QR code by telling the users about what will happen after they scan the QR code. Also, it is important to know that the alternative text of the image is something that should be readable for humans (not like code).
  • The card only needs max-width to prevent the card from becoming too large. So, I recommend removing the width and height. For the height of the card, let the elements inside the card control the height of it. This way, if you adjust the size of the elements inside the card, the height of the card will automatically adjust as well.

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

  1

  ---

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  @Rjae07

  I recommend using the modern CSS from Andy Bell every time you create a new project. I suggest reading the article to learn more about what the CSS code is doing.

  Marked as helpful

  1

  

  Roland Jae V. Patalita•20

  @Rjae07Posted about 2 years ago

  @vanzasetia

  Hello Sir, 👋🏼

  Thank you very much for the feedback. this helps a lot in clearing things out how I will deal with my next challenge.

  I will remember all your feedback in my next challenge.

  God bless always Sir. Thank you a lot!

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  @Rjae07 No problem! 👍

  0