Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

QR code component ui design using flexbox

@purna-dev

Desktop design screenshot for the QR code component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


I use flexbox for this is it ok? Any corrections to the CSS code please let me know?

Community feedback

T
Grace 29,310

@grace-snow

Posted

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-dev

Posted

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

0
mubizzy 1,520

@mubizzy

Posted

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

Fatlind Shehu 2,230

@fatlindshehu

Posted

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-dev

Posted

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

1

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord