Design comparison
Solution retrospective
Some of the easier CSS styles that I couldn't figure out right away - took me longer than I initally expected!
Community feedback
- @PhoenixDev22Posted over 2 years ago
Hi Phúc (Scott) Nguyễn,
Congratulation on completing your first frontend mentor challenge. Your solution looks great. I see you have received some incredible feedback. I have some suggestions regarding your solution:
- Use <main> landmark to wrap the card and
<footer>for the attribution, it should live outside the<main>. HTML5 landmark elements are used to improve navigation experience on your site for users of assistive technology.
- Images must have alt attribute. In my opinion, the alternate text is needed on this image. The alternate text should indicate where the Qr code navigate the user : like
QR code to frontend mentornot describes the image.
- Never use
<div>and<span>alone to wrap a meaningful content. Just keep in mind that you should usually use semantic HTML in place of the div tag unless none of them (the semantic tags) really match the content to group together. By adding semantic tags to your document, you provide additional information about the document, which aids in communication.
- You can use
<h1>for theclass="text-boldand<p>forclass="text-normal".
- In order to center the card on the middle of the page , you can use the flexbox properties and
min-height: 100vhfor the<body>add a little padding to the body that way it stops the card from hitting the edges of the browser. Then you can remove absolute positioning .
width: 350px;an explicit width is not a good way to have responsive layout . consider usingmax-widthto the card inreminstead .
height: 550px;It's not recommended to set height to component, let the content of the component define the height.
- Consider using rem and em units as they are flexible, specially for font size better to use rem. If your web content font sizes are set in absolute units, such as pixels, the user will not be able to re-size the text or control the font size based on their needs. Relative units “stretch” according to the screen size and/or user’s preferred font size, and work on a large range of devices.
- Remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.
- Using percentages for widths, using max-width and avoiding to set heights for the components, with these things is the key to mastering responsive layouts.
Last , It's a good practice to have the styles in separate file he reason for this is that the CSS stylesheet exists for the purpose of defining the presentation style of the document. The HTML file exists to define the structure and content of the document also it's useful If multiple pages on your site have the same look and feel.
Overall, Excellent work! Hopefully this feedback helps.
Marked as helpful1@NationsAnarchyPosted over 2 years ago@PhoenixDev22 Really appreciate the long comment!
These are the tips I was looking forward to, as I already had a bit of experience doing the job IRL (doing maintenance work for clients mostly, not creating new things by myself).
There are definitely a few tips/knowledge you mentioned above, that I have seen before - I just wanted to get this solution quickly done in a short amount of time, as we always know that any codes can still have space for improvement!
1 - Use <main> landmark to wrap the card and
- @correlucasPosted over 2 years ago
👾Hello Phuc, congratulations for your first solution and 😎 welcome to the Frontend Mentor Coding Community!
I've some tips for your:
You don't really need a media query for this challenge, in this challenge all you need is to keep the container responsive, so instead of using media queries, just replace all
widthwithmax-widthto allow the container contract.To manage the qrcode container you don't need to use many divs, keep all content with a single div, just insert all the content with a single div (img, h1 and p) and manage all the padding with this div. This way you have a clean and concise code.
To clean the css, remove all classes and use the direct selector to modify the content, for example (main, p, h1 and img)
👋 I hope this helps you and happy coding!
Marked as helpful1@NationsAnarchyPosted over 2 years ago@correlucas Thank you for your feedback and tips, Lucas!
I found the website through the god of CSS himself on YouTube - Kevin Powell.
As for this, my target for this challenge is to submit the solution as fast as possible, so yes - I can see that my CSS codes aren't quite optimized after reviewing your tips. I will optimize them later, and continue challenging other solutions as well!
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