Multi page responsive w SCSS & JS

Community feedback

• 

  Adarsh Pratap•5,515

  @adarshcodesPosted over 4 years ago

  Hi! @Diarrah, Great work on this challenge👍.

  • Design: your design looks great and everything is working fine.
  • Responsiveness: Your responsive codes are also working properly.
  • Code: I'm just started learning JS, so can't understand the whole code at this moment. But some of the things seem familiar to me. It's clean and well commented. Happy Coding🙂

  4
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  Hey Diarrah, I've just had a closer look at your HTML and there are actually a few recommendations that I've got:

  • You've defined the "Finding the right people..." content in the intro as a h4. I would say this is a paragraph of text. It's not really a heading for anything. So a p would make more sense.
  • Why have you added br tags under each of the h3 headings in the "Build & manage distributed teams..." section? If it's to add spacing I'd recommend just using margin instead.
  • For the testimonials on the homepage and the team images on the about page, you've added alt text for the avatar pictures saying things like "Woman stock photo" or "Man stock photo". I'd recommend being less vague with these and a bit more descriptive to provide more context for screen reader users. Something like "Aiysha Reese avatar photo" would add more context.
  • For the address in the footer I'd recommend using the address element with a ul for the street, city, etc. Here's more information on the address element from MDN.

  I hope these pointers help. As I mentioned before, you've done a really good job on this challenge! 👍

  2

  ---

  

  Diarrah•3,418

  @DiarrahPosted over 4 years ago

  @mattstuddert Thank you for this feedback! May I ask, though, why is it bad to use <br> tags?

  0

  

  Anneta Wamono•60

  @annetawamonoPosted over 4 years ago

  @Diarrah I might be incorrect on this, but I think it's better to use margins for spacing because it gives you more control across different browsers. The br tag may be rendered differently on different browsers.

  2

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  @Diarrah @annetawamono has hit the nail on the head with this. margin is a much more accurate way to space out elements and doesn't add any unnecessary HTML.

  0
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 4 years ago

  Hey Diarrah, you've done an awesome job on this challenge. Everything looks amazing and works as per the brief. Your JS works well too and is well written and commented.

  Great work! 🙌

  2
• 

  Esu Gabriel•260

  @orion-esuPosted over 4 years ago

  Super Amazing

  1