Hi
Have you intentionally made this look totally different to the design? The goal of the challenges is to build out the supplied design just like in a real job, so this wouldn't be accepted in those circumstances I'm afraid.
You need to make some changes (as well as adjusting the styles to match design and using the supplied assets)
- Use landmarks. Every page should at least have a main element
- Once you are using the correct image, it does not need wrapping in a div. Only the card itself needs padding on this
- The alt description should ideally be more meaningful. A QR code is a scannable image, so say where the code goes to. Imagine a real world scenario where this component would be used - There could be multiple cards on one page, so it wouldn't work if every image was described only as "QR code" "QR code" "QR code"
- The card should not have an explicit width, only a max-width. And this should be in rem, not px. This will allow the component to work on all screen sizes, even if users change their text size, as it allows the card to remain proportional to that text size and allows it to shrink when needed without overflowing the screen.
- Font size must never ever be in px. Use rem. This is extremely important so that user text size preferences can be respected.
- You appear to be confusing margin and padding a little in this but part of that may be due to deviating from the design. I would expect the card to have padding as that is for internal spacing - keeping the child elements from touching its edges - and the child elements to only have vertical margins to create external space between each element
- Is there a reason you've used a heading level 3 in this? (Not necessarily wrong, but depends on your reasoning)
- This completely breaks on mobile landscape because you have set a wrapping element to
height: calc(100vh);
. This limits the height of the body and means the card gets positioned off part of the screen, making the QR code unusable. Rule: Never limit the height of elements that contain text, and hardly use height at all. The only exceptions I can think of are for icons or some images. Let the browser decide on the height required for the content. This would be fixed in your case by changing height to min-height, allowing the body to grow and extend beyond the screen height when needed. - Make sure either the component has a little margin or a wrapping element has a little padding. This will stop it hitting screen edges on some devices.
- Always include a CSS reset at the very start of a stylesheet. Look up the modern CSS reset by Andy Bell which has a good explainer blog about it
I hope this is helpful. These are all really important foundations you need to start building into every project
Marked as helpful
@Wali1209
Posted
Hi! @grace-snow Thank You very much for your Valuable Feedback! Yes, I do make this look different deliberately! Yeah Definitely in the real job, it can't be acceptable, and I will make changes to it to match the design.
Actually, I want to use (https://goqr.me/api/) API in my challenge to generate a QR Code image, it generates QR Code with a different color so I do make this look different but it's not quite Intentional!
@Wali1209
Posted
Nope! I have no solid reason for using the h3 tag, It seems h3 heading in the sample design I didn't think of it much TBH. And I am gonna make changes according to your feedback and let you know when I'm done. Your Feedback helping me a lot, hats off to you @grace-snow!