Four Cards - Sass & BEM & Grid

Community feedback

• 

  Loris DUCAMPS•425

  @LorisDucampsPosted almost 4 years ago

  Hi SzymonRojek,

  Very good work ! it is good for the eyes !

  After inspecting your work, here is my feedback:

  Pay attention to the size of your img (icons). After looking in the inspector, they are not 64x64.

  To remedy I invite you to apply the following code:

  img { display: block; height: auto; max-width: 100% }

  without forgetting to remove the width and height of your img in your class (..grid__item-img)

  I invite you to go and see my design which is not perfect but can support my point :)

  https://four-card-feature-section-inky.now.sh/

  I hope I helped you.

  Do not hesitate if necessary

  Good luck

  1

  ---

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @LKA-LORIS

  thanks for comment :D

  I have checked my code. I started project from mobile-first with 57px and then on desktop I want to get 64px max-width and height - in my opinion I do not need these properties height: auto; and max-width: 100% but thanks for a suggestion.

  Greetings

  0
• Account deleted

  Incredible, it looks the same ! Congrats

  1

  ---

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @emmanuelniasse

  Thanks for comment :D

  Greetings :D

  0
• 

  T

  Grace•31,250

  @grace-snowPosted almost 4 years ago

  Whoop, nice one!

  I'd say this is very nearly perfect! Just one thing missing.... text-align: center; on your .header__heading to center it when it wraps on mobile

  good job

  1

  ---

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @grace-snow

  Thank you so much. That's true. Next time I have to use more precisely the responsive tool from the inspector in my browser because the heading was good "mostly" on all mobiles.

  Super! :D Thank you for your support.

  0
• 

  Leon•180

  @leon-bwPosted almost 4 years ago

  Amazing solution! Looks perfect to me :D

  1