Design comparison
Solution retrospective
I'm fairly happy with the way this turned out. Please feel free to give any suggestions!
Community feedback
- @BrianJ-27Posted over 2 years ago
Hey Kpax
Overall the layout looks really good visually. You matched the box shadow perfectly on the cards and I may have to steal that color from yours and add it to mine! :) Also great job in not getting any accessibility and html errors. When I checked out your code, there a few things I noticed:
-
Avoid using fixed widths and try using more flexible widths like % or rems. "ex) width 1100px" The only time I would want to used a fixed width is if it was for something intentional. So for example, I only want my card to be a maximum width of 400px so in this case, instead of using width, I would use, "max-width: 400px". This is better and more flexible because your card's width will adjust to the screen until it reaches 400px and then it won't grow no more. Also any where you are using px's see if you can't convert to rems unless you intentionally need it to be px's.
-
I see you are using Sass which is awesome and I noticed you are still using the @import directive. Very soon this will be deprecated and be replaced with @use & @forward. If you need some help in implementing it, on Youtube, Kevin Powell provides a great breakdown on how to do this. Here's the link. https://www.youtube.com/watch?v=CR-a8upNjJ0&t=10s
-
You have unused styles in your style.css. To be more specific, on your .card-container class on lines 119-126 you don't need, flex-direction: row (on line 121) because display: flex will default to a row direction.. Also you don't need "margin: auto & width: 1140px" (on lines 124 & 126) because you already defined "justify-content: center" which is centering your content horizontally, so now we just cleaned up your css a bit. :) So my suggestion would be to take a moment and refactor your code and see if you can make it more cleaner.
But overall great job
Marked as helpful0@kpax10Posted over 2 years ago@BrianJ-27 Thank you very much for these suggestions. I haven't been able to get my head wrapped around the @import and @use, even after watching some videos on it. I've heard some mumblings that the they won't be deprecating @import, so hopefully thats the case. Thanks again for all the tips!
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