Design comparison
Solution retrospective
Open to feedback for improvement. Thank you!
Community feedback
- @MelvinAguilarPosted almost 2 years ago
Hello there 👋. Good job on completing the challenge !
I have some suggestions about your code that might interest you.
HTML 📄:
- Use the
<main>tag to wrap all the main content. With this semantic element you can improve the accessibility of your page.
Alt text 📷:
- The
altattribute should not contain the words "image", "photo", or "picture", because the image tag already conveys that information.
-
The
altattribute should explain the purpose of the image. Uppon scanning the QR code, the user will be redirected to the frontendmentor.io website, so a betteraltattribute would beQR code to frontendmentor.ioIf you want to learn more about the
altattribute, you can read this article. 📘.
CSS 🎨:
- There is no need to include the <html> element to center the whole component, instead of using
html, body { }use onlybody { }
- To center an element vertically, you should use a height to its container. In this case it is recommended to use "min-height: 100vh" so that it occupies 100% of the viewport height. e.g.:
body { min-height: 100vh; }
- Don't use units like vh for font-size (unless you use
clamp()). This can make the font size too small on mobile devices and too large on desktop devices. With font-size use relative units likeemorrem. You can read more about this here 📘.
I hope you find it useful! 😄 Above all, the solution you submitted is great!
Happy coding!
Marked as helpful1@Pedro-CelestePosted almost 2 years ago@MelvinAguilar Hey there! I'm curious, how can you make break lines <br> in markdown after bullet point items? Thanks 😅
0 - Use the
- @Pedro-CelestePosted almost 2 years ago
Hey @@tjg1093! 😎👍🏻
Congratulations on finishing your first frontend mentor challenge!
Here are some things you could change to make your code even better:
HTML:
-
Every page should be contained in at least one semantic element. This is really important stuff when it comes to accessiblity. Instead of using
<div class="card">as your main parent element you should use<main>. See more about semantic HTML here. -
When writing ant alternative text for the
altattribute of an<img>element, don't include words like image, photo or picture, since screen readers already announce it like one. Instead ofalt="qr-code image"you could write something likealt="QR Code that goes to the Frontend Mentor website.".
CSS:
-
It's generally a good idea to use
border-boxas the box model of your web page. This can save a lot of time in more complex projects since it simplify the way you calculate padding, borders and margins. You could achieve it like this:*, *::before, *::after { box-sizing: border-box; } -
In the rules for the
<body>element, instead of usingmargin: 60px 10px 100px 10px;which is not useful when you think that users with completely different screen sizes are going to access your website, usemin-height: 100vh;. This makes the webpage have a minimum size equal to the viewport, completely centering the card with the flex properties you set. -
When defining sizes in your web page, relative units can make it much more responsive and accessible, if you are interested in learning more about it, here's an awesome article that explains why an how.
I know that's a lot of things, but this stuff can really makes the difference you know. 😅
Hope to see more of your code here soon! Have a nice one :)
0 -
Please log in to post a comment
Log in with GitHub
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