CSS, CSS Animation, Flexbox, Responsive Layout

Community feedback

• 

  

  ApplePieGiraffe•30,525

  @ApplePieGiraffe

  Posted over 4 years ago

  Hi there, Folarin Akinloye! 👋

  Nice to see you complete another challenge! 😀 Good work on this one! 👍 Your solution looks good and responds up/down pretty well!

  I'd like to suggest,

  • Setting text-align to center for the heading and the description text in the mobile layout of the site (as in the original design).
  • Like ctnance mentioned, adding the background images from the original design using CSS background images.
  • The way you named your CSS classes looks pretty similar to BEM (just add an extra underscore between terms and you'll be following BEM, too). 😉

  It's good to see you remembered to leave out the alt text for the stars images (so that screen readers don't repeat themselves a whole lot). 😀

  Keep coding (and happy coding, too)! 😁

  22
• 

  

  Grace•32,130

  @grace-snow

  Posted over 4 years ago

  Hey, looks like you've already had lots of great feedback here so I won't repeat ☺

  One small but important design point looking at your desktop preview. Do you see how in the original, the top and bottom sections align with each other on the left and right? In yours, they don't align on the right. These kind of details drive designers nuts and will be flagged as bugs by a test team, so it's good to get into the habit of noticing and fixing them now.

  I hope this is helpful advice

  11
• 

  Christopher Nance•240

  @ctnance

  Posted over 4 years ago

  Hi Folarin, great job on this one!

  I noticed you used several media queries for the same min-width; I'm not sure if this was intentional, but you could use only one for the same effect to trim down your code.

  Also, don't forget you can add the background images as well with the background-image attribute.

  Again, really good work--keep it up!

  13