Responsive Four Card Section with Sass

Community feedback

• 

  Brian Johnson•210

  @BrianJ-27

  Posted over 3 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:

  1. 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.

  2. 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

  3. 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

  01

  Marked as helpful