@thibault-deverge
Posted
Hi Francisco,
Congratulations on completing your project! Overall, it's really well done and closely matches the original design. It's evident that you've put a lot of effort into this, especially if you were working under a tight deadline. Bravo! 👏
Here are some specific points as I'm still here to feedback and found some improvements:
Visual Feedback:
-
The cards are missing the full white background. Currently, they have a grayish tint, but the original design specifies a completely white background.
-
The content within the cards is centered both vertically and horizontally. However, the design only centers it horizontally with a larger margin at the top. Maybe adjust the vertical alignment to have more top margin instead of centering vertically.
-
The cards appear slightly wider and less tall than the original design.
Overall Visual Impression: I'm quite peaky because your design is already really close to the original. So that's no deal breaker things to fix.
About code & SCSS Organization:
-
SCSS File Structure
- Organizing SCSS files helps in maintaining and scaling your styles. Maybe in the future structure your SCSS with separate partials and import them into a main file. There should be one main file generally 'main.scss' which would import all the other one.
/scss │ ├── abstracts/ │ ├── _variables.scss │ ├── _mixins.scss │ │ ├── base/ │ ├── _reset.scss │ └── _base.scss │ ├── components/ │ ├── _card.scss │ │ └── main.scss // Main file importing all partials
- Organizing SCSS files helps in maintaining and scaling your styles. Maybe in the future structure your SCSS with separate partials and import them into a main file. There should be one main file generally 'main.scss' which would import all the other one.
-
BEM Naming Convention
- Using multiple classes like
feature__card feature__card--blue
enhances clarity and follows BEM methodology. Apply BEM consistently to maintain a clear and scalable structure. In your code for example, you use differents class for all the article even if most style will be applied for all.<article class="feature__card feature__card--1"> <!-- Card content --> </article>
- Using multiple classes like
-
Unit Usage
- I found thare an excessive use of
px
for padding and other properties. Userem
orem
for padding and margins to ensure scalability across different screen sizes. Keep 'px' for really close mesure like border-radius, box shadow and for all the rest -> em/rem. I could suggest to use an online converter to convert from px to rem or better, a vs code extension ! :)
- I found thare an excessive use of
-
CSS Grid
- You've effectively used Flexbox for the desktop layout, which is impressive to realize this layout with only Flexbox! For future projects, consider exploring CSS Grid as it offers powerful capabilities for complex layouts and might simplify some aspects of grid management. For this kind of design, it can really simplify your life you'll see. Just need a 12 layout column grid and align based on that
Overall Code Impression: Your code is clean and functional. That's really good work and really impressive for the desktop design with only flexbox !
Happy coding and best of luck with your next project! 🚀
Marked as helpful
@antoniomontoia
Posted
Thanks a bunch for the feedback, these are quite valuable pieces of advice! @thibault-deverge