stats-preview-card

Community feedback

• 

  Gabriel Montplaisir•210

  @GabrielMontplaisirPosted almost 2 years ago

  Hey Ruffwise!

  Nice code to a challenge I attempted myself just a few days ago. I like your solution for the margin a little better than mine, and may even "steal" some of your ideas for mine!

  I noticed some stylistic differences between your product and the design, and thought I may share some ideas. Take them as you please, as I'm learning as well:

  • You didn't use the desktop version of the picture at all. Typically, this is preferable due to resolution reasons. The more you scale a small picture, the more distorted it becomes. You can achieve this by replacing your <div class="image-container"> w/ <picture media="(min-width: 900px)" srcset="/path/to/image-header-desktop.jpg" />. You can then replace your CSS appropriately from .image-container to picture. Your alt text also needs to be more descriptive to accurately reflect what the image displays.

  • The font-weight for .stats should be the same as your h1. So font-weight: 700.

  • I noticed you repeat yourself a lot between your two different media queries (for example for your .picture, .container, etc.). You only need to mention the changes once, as the values from the second media query will inherit the values of the former. Since you're working from smaller screens to larger ones (using min-width), that means you ONLY need to mention .picture, .container, etc. in the min-width:900px media query UNLESS you're changing the value again when it's min-width:1000px.

  Hope this helps a little! Good luck!

  Marked as helpful

  0

  ---

  

  ruffwise•50

  @ruffwisePosted almost 2 years ago

  @GabrielMontplaisir Thank you so much for your feedback, regarding the desktop picture - i totally forgot there's a desktop version of the picturecovers face, also, thanks for the other tips/ideas, I will incorporate these changes and apply them accordingly. Thanks again for the feedback, it's very much appreciated.

  0