Hi
I see a few issues here that could do with improving
- The design has a heading, but you have not used a heading element
- 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)
- 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
- 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
- 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
- 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.
- The image should be display block and width 100%. It should not have an explicit width or height.
- 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
- 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.
- I recommend placing the attribution in a footer element outside of main.
I hope this is helpful
Marked as helpful