QR Code Component

Community feedback

• 

  Kamlesh Rajesh Yadav•5,060

  @Kamlesh0007Posted over 1 year ago

  Congratulations on completing the challenge! That's a great achievement, and I'm sure you put a lot of effort into it. I really liked the way you approached the challenge and the code you wrote. You demonstrated a good understanding of the concepts and applied them effectively to solve the problem.I have a few suggestions to improve your code further. When it comes to centering a div or any element on a webpage, using margins may not always be the best approach. so use flexbox or grid layout for centering the div. make the container center properly use min-height:100vh which is used to ensure that a container or element takes up at least the full height of the viewport (the visible area of the browser window) regardless of the content inside it.

  Here's an example code snippet:

  body {
  display:grid;
  min-height:100vh;
  place-items: center;
  }
  
  now remove below lines of code
  .qrcode-card {
  margin: 3em auto;
  }
  

  Marked as helpful

  1

  ---

  

  Gabriel•170

  @ctrl-brokencodePosted over 1 year ago

  @Kamlesh0007

  Thank you for your suggestions! With this, I finally learned how to center a div or element! Thank you again for taking your time to help me. Hope I see you again some time!

  0
• 

  T

  Grace•31,090

  @grace-snowPosted over 1 year ago

  Hi I have some recommendations for you

  1. the bigger text should be a heading element
  2. make the alt text on the image clearer. QR code to where?
  3. don't style on IDs. That's not what they are for. Use classes for styling instead
  4. the component should not have a width, only a max-width in rem
  5. the component must not have a height. Never limit the height of elements that contain text. The height will be determined by the content automatically
  6. the image should be display block and width/max-width 100%. It shouldn't need margin except possibly margin bottom. Usually this is all done anyway as part of a modern css reset (which you should always include at the start of the styles anyway)
  7. the component should have padding
  8. to center the component on the page, use flex or grid properties on the body along with min-height 100vh
  9. use the font size in the style guide but converted to rem for the body (and therefore paragraph which will inherit the size)
  10. I doubt the heading and paragraph will need to be width 80% after making all these changes. Usually you would give elements like that a max-width in ch but it's fine to set a % width if you prefer. It just might need to be set higher than 80% once the card has correct padding

  Marked as helpful

  1

  ---

  

  Gabriel•170

  @ctrl-brokencodePosted over 1 year ago

  @grace-snow

  Thank you for the recommendations! I definitely learned and remembered some things now. I made these adjustments and I would like to ask, if you could and have time, to take a look again at my code and see if I missed something (probably yes, haha). I'd like to thank you again for taking your time to help me!

  0