Social proof section with SASS

Community feedback

• 

  Hector Lopez•195

  @hec-lopzPosted over 3 years ago

  Hi there!

  It looks great! Congrats on your solution. 🎉

  I'm not sure if you used the HTML <article> and <section> tags in the best way possible. It is my understanding that an <article> is a section of your pages that works by itself and can be replicated anywhere without loosing sense. A <section> represents a section separated from the others, but still requires its context to work. Given this, your .card-container and .rating-container should be sections themselves and not the title of each card, those titles could´ve been <h2> tags.

  No matter what, I really liked your solution! Hope you keep it coming and don´t hesitate to share your solutions. 😄

  1

  ---

  

  Agustina•155

  @aguscorvoPosted over 3 years ago

  @hec-lopz hey! Thanks for the comments. Sorry, I don't know if I fully understand. You meant that .card-container and .rating-container should be sections instead of divs, didn't you? They were sections before I made changes because of HTML issues. Should the card-header be a div instead? "A represents a section separated..." Were you talking about articles?

  Sorry for all these questions, it's just that I want to understand your feedback. Thanks in advance :)

  0

  

  Hector Lopez•195

  @hec-lopzPosted over 3 years ago

  Hi there @aguscorvo!

  I'm not sure why it didn't show up, but yes, I was talking about your <article> tags.

  "... an <article> is a section of your pages that works by itself...".

  "A <section> represents a section separated from the others..."

  And yes, I meant that your containers should be sections and not divs, that's my opinion at least.

  No worries, whatever I can do to help. :)

  0
• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted over 3 years ago

  Hi, Agustina Corvo! 👋

  It's nice to see you complete your second Frontend Mentor challenge! 🎉 Good job on this one! 👏 Your solution looks great! 👍

  I'd like to suggest,

  • Switching to a mobile-friendly layout a little sooner to prevent a horizontal scroll bar from appearing along the bottom of the page.
  • Making sure the background image in the top-left corner of the page sticks to the top of the screen (currently, it moves down a bit when the height of the screen changes in the desktop layout).
  • Trying to include a heading wherever you use the <article> or <section> tags to identify them in order to improve the semantics and accessibility of your page. 😉
  • Making sure the favicon for the site shows up (perhaps double-check the file path to the image).

  Keep coding (and happy coding, too)! 😁

  1

  ---

  

  Agustina•155

  @aguscorvoPosted over 3 years ago

  @ApplePieGiraffe Thank you very much for all the feedback.

  • I took a mobile-first approach but of course I may have made mistakes.
  • I fixed the three last points :)

  Always happy coding :D

  1

  

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted over 3 years ago

  @aguscorvo

  Awesome! Just checked again, and those changes looks good! 👍

  1