A few important foundational learnings:
- linking fonts in the html head is slightly better for performance than using imports in CSS.
- Don't ever limit the height of elements that contain text, including the body. Use min-height instead so the body can grow taller than the viewport height when needed (eg on mobile landscape).
- Never set font size in px.
- if using flex on the body it should be a column not row.
- don't set a width on the component. It's making it hit my screen edges. Instead, set a max width only. And use rem instead of px so the layout works for all people and scales correctly even if those people use a larger text size.
- make sure the component can never touch screen edges by either giving it a little margin or giving a wrapping element (eg body) a little padding.
Marked as helpful
@ashap-bappy
Posted
@grace-snow Thanks for your valuable feedback. Will try to improve my solution.
@ashap-bappy
Posted
@grace-snow I've updated my code according to your suggestion. Can you please review it?
@ashap-bappy that looks good now. The only change left to fix is the alt description on that image. Remember alt text is content not code. It's not a class name. It's an actual description of the image. In this case it needs to say what the image is (QR code) and where it goes to (to FrontendMentor.io).
Marked as helpful