Design comparison
SolutionDesign
Community feedback
- @gmagnenatPosted 3 months ago
Hi, congrats on completing the challenge!
Here are a few pointers to help you improve your solution and tackle future challenges:
Does the solution include semantic HTML?
- You are missing a
<main>
landmark in this solution. It's needed to indicate the main content of a web page; it should wrap the main content. - The image doesn't really need to be wrapped in an extra
<div>
here if you want to simplify your HTML. - Attribution needs to be in a landmark, so you can use the
<footer>
here. - The
alt
attribute of the QR code should be a bit more descriptive. For example, "QR Code to frontendmentor.io."
Is it accessible, and what improvements could be made?
- You should use
min-height
instead of a fixed height on your container in case the content takes up more space. With a fixed height, your layout can be cropped at the top and bottom. - Don't set a fixed height on your card; let it scale with the content. The content can take up more space if the user increases the browser default font size, for example.
- Use relative units in your
max-width
value instead of pixel values. This helps when the user increases the browser default font size. With pixel values, the size cannot adjust to the updated root font size. - You don't need to set a width or height on the image. It's contained in the card and should scale with the card width.
- You should not use pixels for anything related to fonts, as they need to respect user settings. You can read this great article that explains everything in depth: Why font-size must NEVER be in pixels.
Does the layout look good on a range of screen sizes?
- You should not use
position: absolute
on the attribution. On some screen sizes, it can be displayed over the card itself, which is not ideal.
Is the code well-structured, readable, and reusable?
- You need to add a modern CSS reset at the top of your stylesheet in all your projects. It will help you deal with browser inconsistencies and allow you to start on a clean slate that works similarly across different browsers.
- You shouldn't style HTML components directly. Instead of targeting your
<h1>
, for example, just add a class and style the class. Generally, you want to aim for the lowest CSS specificity and increase it only if needed. This approach also helps you maintain your code better; if you later want to change this<h1>
to an<h2>
, you won't need to update your CSS.
Does the solution differ considerably from the design?
- It looks good and respects the design intention.
I hope you find something useful in this list. Don't hesitate to ask if any of the points aren't clear.
Happy coding!
Marked as helpful0 - You are missing a
Please log in to post a comment
Log in with GitHubJoin 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