Mobile first, HTML, SCSS, Flexbox, BEM

Community feedback

• 

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted over 3 years ago

  Hi there, Juan Bonilla! 👋

  Great work on this challenge! 👍 Your solution is looking good and its responds well! 👏 I like the intermediary layout you added between the mobile and desktop layouts! 🙌

  Just a small tip to make your HTML more semantic—I think you can use <article> tags for the testimonial cards and heading tags for the headings inside them (rather than mostly <div>s for everything). 😉

  Also, I would suggest leaving the alt text for the star icons empty (so that screen readers won't ready out the alt text for every star, which can be a little annoying and isn't necessary). 🙂

  Keep coding (and happy coding, too)! 😁

  1

  ---

  

  Juan Bonilla•610

  @juanpb96Posted over 3 years ago

  Hi, ApplePieGiraffe! Thanks for your comment and your suggestion!

  Now that you mention a tip about my HTML, I wonder if taking the name and subtitle of my testimonials could be wrapped inside a heading tag or what do you consider should be the heading in every testimonial?

  On the other hand, your tip about the alt text is great! I didn't think on this. I'll be more cautious when using this alt attribute.

  Happy coding!

  1

  

  T

  ApplePieGiraffe•30,545

  @ApplePieGiraffePosted over 3 years ago

  @juanpb96

  Glad to help! 😀

  About which element(s) should be the heading inside each testimonial—I suppose you could go wrap both the name and title of each card in a heading tag (since they make sense together). 🙂

  1

  

  Juan Bonilla•610

  @juanpb96Posted over 3 years ago

  @ApplePieGiraffe

  Perfect! I will change that in my HTML.

  Thanks for your suggestion!

  0