QR code component ui design using flexbox

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  Hi

  I see a few issues here that could do with improving

  1. The design has a heading, but you have not used a heading element
  2. The paragraph of text has been placed in a span instead of using a meaningful element. Never have text in divs or spans alone, always use a meaningful element (like a paragraph)
  3. The image is the most important content on this whole component but you have treated it as if it is decorative. It must have an alt description saying it is a QR code and its destination
  4. On my mobile, the top of the image is cut off completely, meaning the QR code doesn't work. This is because you've used height 100vh. This should be min-height instead so the content can extend beyond the viewport height when it needs to
  5. The card should not have a height. Never give components that contain text like this a height. There is no need and it can only cause bugs for people who change their font settings
  6. Similarly, the card should not have a width. It should have a max-width instead. This will allow it to shrink if needed on small screens.
  7. The image should be display block and width 100%. It should not have an explicit width or height.
  8. Padding serves to create space from the edges, not margin. You don't need to put padding on the text wrapper or margin on the image. Instead just give the whole card some padding. Simple one line of css. Then inside the card, all you need to do is set vertical margins to create space between elements
  9. The font size on your solution is slightly smaller than the style guide. I definitely think you should be using rem not em. Em inherits from a parent so should only be used in very specific circumstances (and rarely for font size). It is more often used for vertical margins or padding on buttons so it scales with the font size.
  10. I recommend placing the attribution in a footer element outside of main.

  I hope this is helpful

  Marked as helpful

  1

  ---

  

  Purna Chandra Saren•40

  @purna-devPosted over 2 years ago

  @grace-snow thank you I will do corrections in my code

  0
• 

  mubizzy•1,520

  @mubizzyPosted over 2 years ago

  Excellent job on this challenge! your report has a few issues though:

  1. wrap everything in your body in <main> or use semantics

  2. it is a best practice to use both HTML 5 and ARIA landmarks to ensure all content is contained within a navigational region.

  Hope it helps:)...don't forget to mark it as helpful 👍

  Marked as helpful

  1

  ---

  

  Purna Chandra Saren•40

  @purna-devPosted over 2 years ago

  @mubizzy thank you

  0
• 

  Fatlind Shehu•2,230

  @fatlindshehuPosted over 2 years ago

  Hi there,

  Have you done good using flexbox? Absolutely, flexbox is perfect for this kind of situation, also I think you did a great job using em units, for the responsive design they're great, I would recommend reading more about rem and em, I like to use rem because of their base font-size em seem to get complicated some times.

  Keep it up & happy coding...

  Marked as helpful

  1

  ---

  

  Purna Chandra Saren•40

  @purna-devPosted over 2 years ago

  @fatlindshehu thank you for your feedback and I will definitely look into rem and em

  1