Design comparison
Solution retrospective
If you have any comments or ideas on how I can make my code better or the design look better please share your thoughts with me.
Community feedback
- @tmerrick17Posted about 3 years ago
Hey Amr Fouad, first of all a great go at this project. Here is some feedback I would love to give you. I downloaded the files and took a deep dive so here it goes.
Things that worked well:
- CSS Reset is always a good idea. The basics you have exactly there.
- Custom CSS3 variables. Another good one that is so helpful, especially if you look at working in SCSS/Sass.
Things to ponder:
-
Best practice is to leave out unit (eg. 0 instead of 0px). This is nit-picky but a good standard to show other developers that you have the basics down.
grid-column-gap: 0px;
turns into `grid-column-gap: 0; -
Definitely try to practice mobile first (look up Youtube videos, here's a good example.
-
Also when you hit 769px at that spot, the card is half out of the viewport. Try something around 1200px?
-
Try using px rather then % for
border-radius:
. Start with 20px and go from there? -
What if you took off
.parent .div1
? Try to not nest these already customized classes you've created. See what happens on the webpage, hopefully the answer is nothing. This could save you time writing code you might not need to type out.
Example:
.parent .div1 .txt h1 { font-size: 1.6em; padding-left: 0; }
...turns into...
.txt h1 { font-size: 1.6em; padding-left: 0; }
- Try using more descriptive classes then div2 but general like card-image? Too specific would be purple-image-of-people. Hopefully you get the idea.
Ok, I know that was long but I wanted to be as thorough as possible. You have done a great job at getting this far. Going back and tweeting while it might feel sucky, it means you are growing in your coding. So am I. I've only been coding for a a year now and really have enjoyed it. Hope to see more of your work!
Marked as helpful0 - @thisisharshjainPosted about 3 years ago
Heyy @A-Fouad-Code, I'll point out a few things here,
- Main card isn't centered instead on top-left side
- Color overlay on image doesn't looks that good. maybe change the opacity a little
- Try to use semantic elements when possible
- Reducing the padding-top on .stats will look better
- It kinda breaks between 770-1199px Except these few small issues, You've done a good job! 😄 Kepp grinding 💪🏻
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