Design comparison
Solution retrospective
While it was a simple challenge, there’s not much to be particularly proud of. However, I’m happy with how I used CSS variables to organize the code and ensure a more maintainable structure. I also focused on achieving a pixel-perfect design, which I was able to replicate quite well. For now, I wouldn’t do anything differently.
What challenges did you encounter, and how did you overcome them?The main challenge was ensuring the design was pixel-perfect. While it wasn’t difficult, I had to constantly go back and forth between the code and the Figma file to match the design closely. This process helped me refine my attention to detail.
What specific areas of your project would you like help with?I’d like feedback on what areas could be improved, particularly if there are any mistakes or bad practices in my code that I may not have noticed.
Community feedback
- @StroudyPosted about 2 months ago
Exceptional work! You’re showing great skill here. I’ve got a couple of minor suggestions that could make this stand out even more…
-
While
px
is useful for precise, fixed sizing, such asborder-width
,border-radius
,inline-padding
, and<img>
sizes, it has limitations. Pixels don't scale well with user settings or adapt to different devices, which can negatively impact accessibility and responsiveness. For example, usingpx
for font sizes can make text harder to read on some screens, Check this article why font-size must NEVER be in pixels. In contrast, relative units likerem
and adjust based on the user’s preferences and device settings, making your design more flexible and accessible. Usepx
where exact sizing is needed, but prefer relative units for scalable layouts. If you want a deeper explanation watch this video by Kevin Powell CSS em and rem explained. Another great resource I found useful is this px to rem converter based on the default font-size of 16 pixel. -
Line height is usually unitless to scale proportionally with the font size, keeping text readable across different devices. Best practice is to use a unitless value like
1.5
for flexibility. Avoid using fixed units likepx
or%
, as they don't adapt well to changes in font size or layout.
I hope you’re finding this guidance useful! Keep refining your skills and tackling new challenges with confidence. You’re making great progress—stay motivated and keep coding with enthusiasm! 💻
Marked as helpful0@thibault-devergePosted about 2 months ago@Stroudy Thank you so much for the really helpful feedback! 😊 The resources you shared, especially the article on font-size, Kevin Powell's video, and the px to rem converter (which I’ve already bookmarked), were incredibly insightful. I’ve implemented all your suggestions—switched to using rem for scalable layouts and set a unitless line-height for better readability. I’ve also made sure to structure everything using the BEM methodology for clearer and more maintainable code.
Thanks again, and good luck with your next projects!
0 -
- @geomydasPosted about 2 months ago
I've just finished reading your code and it seems good but it has some few mistakes. Don't worry as most of these mistakes can be easily fixed and are common for beginners. I've done this in the past aswell and not everyone is perfect.
My Tips and Feedback
- Use a code formatter such as Prettier. Manually indenting and tabbing is inconsistent, inefficient, and less maintanable. Using a code formatter does it consistently and automatically with just a keyboard shortcut
- No need for those section tags, it's all in one card anyways. You would typically use a section tag for certain recognizable parts of page that have headings or so which is not the case here..
- Never ever set font-sizes in px. That's an accesibility issue and you should use the rem/em unit instead. See why 4.Don't overuse nesting and descendant selectors. You would typically use a descendant or nesting selector for general styles such as in a CSS reset. Adding a class name for each element makes your CSS infinitely more readable and understandable aswell. You can take a look at the BEM methodology and try to implement it inside your projects and see if it works for you.
- Remove the min-content and 90vw in the .card div width and max-width. Those 2 properties contradict each other making your max-width useless. Just set a
width: 100%
instead and set a max-width in fixed units such as rem or px and you will be good to go. - Use the rem unit in place of px most of the time. You would typically use the px unit for very small stuff in the design such as borders, outlines, shadows, border-radiuses, and etc. You would use rem for the rest. To be specific, you would use rem if you want it to scale with the user's set font size in the browser's setting which is the case most of the time.
There's a few more issues but I reccomend you do the pointers above first so you won't be overwhelmed. Have a nice day and have fun coding!
Marked as helpful0@thibault-devergePosted about 2 months ago@geomydas Wow, I wasn’t expecting such detailed feedback for an early project! I can really see how valuable this platform is for improving my skills, and I’m super grateful for all the advice you gave me. 😊
I’ve implemented all your suggestions, like switching to rem for accessibility, cleaning up the card width with max-width in rem, and getting rid of the unnecessary section tags. I'm already using Prettier and yes that's a game changer ! Plus, I’ve started using BEM for better structure and readability. Thanks for the resource also, that was really interesting !
Thanks again for taking the time to help me out—it’s made a huge impact on my project. Wishing you all the best in your next projects, and happy coding!
0@geomydasPosted about 2 months ago@thibault-deverge Seems fine but you can remove the .card_image div and instead just apply the styles directly on the image. Don't see why you would need overflow: hidden. The CSS reset that you have been using (meyerweb) is quite old and you should be using more modern ones which are Josh Comeau's or Andy Bell's CSS reset. I personally prefer to use the first reset.
You can change the indentation in Prettier to specify how long each indentation/tab is. It's personal preference though but I think it should be 2 at most.
No need to use flexbox for everything here. In the design, the gaps are unequal and you should use margins instead so remove that .card_description div and instead apply margins on each element individually
Marked as helpful0@thibault-devergePosted about 2 months ago@geomydas Thanks so much for your helpful feedback!
I’ve gone ahead and removed the extra div. The issue I was encountering with smaller screens is now resolved, and without the unnecessary div, the overflow: hidden is no longer needed. This also fixed the spacing issue caused by the image not filling the container properly.
I appreciate the suggestion about using Josh Comeau’s CSS reset—it was an insightful resource, and the article that came with it was very informative.
Regarding indentation, I realize it’s a matter of personal preference, and for now, I stick with what I’m used to in my personal projects. However, I completely agree that when working in a team environment, I will follow the project’s established conventions.
Once again, thank you for your feedback!
0
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