Yahia ElGamily• 370
@YahiaG
Posted
Hi Silvy,
I really like how organized and awesome your code is. I'm checking solutions to enhance my coding skills. I allowed myself to fork your solution repo :) and here are some good points I like about your code:
- Using comments well to describe every element also as a draft for styling guides
- I like when you converted rem into 10px unit to make it easy to use
- As I said your styling is organized ( global rules, components like buttons then the normal flow)
- Splitting MQ into different sheet is nice However, I prefer to keep it beneath every element to easy track them
Here are some points to make my comment useful :)
- I didn't understand why you used max width for container then you changed into using margins for every MQ. I noticed container isn't centered at width less than 78em (1248px) and greater than 1200px. I saw some solutions using container width 90% and It worked.
- Media Queries are annoying- I agree- but I see you started your design with desktop width and that's against the concept of mobile first. check this post for Grace Snow, she is perfect with that.
- I see you included design files, style guide and readme-template in your repo. it's ok but, it's good to exclude them (add them to .gitignore file)
- Also customize README file to document and introduce your project on github, You'll need to use Markdown language. it's awesome and easy to learn.
Finally, I hope my commnet isn't annoying. I'll keep following your solutions to learn more from you. Thank you, Yahia
Marked as helpful
1
