Testimonials Grid

Community feedback

• 

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  Hi Vaishnav,

  Wellcome here, well done :D

  A few tips below:

  • check your HTML report issues above and try to fix it;
  • instead of div class="container" you can use the main tag;
  • instead of the h2 I'd suggest using the p tag because it is not a heading => bold text doesn't have to indicate on heading;
  • this is a single page component so the h1 can use with the class sr-only set to hidden visible only for the Screen Readers;
  • alt text should be added => just write name of a person because these images don't have a decorative role;
  • I'd suggest learning the BEM naming convention, especially that you have used the grid;
  • RWD: check a project in your browser by suing the inspector on different devices => add margin top and bottom; also, you can create one @media breakpoint for tablets by adding another design pattern: two rows with two boxes each and below the 5th box (named Kira) along the length of the entire container (otherwise all content looks very tight);
  • I think you should add max-width 1440px, also in my opinion max-width 600px doesn't have to be added to the cards;
  • add border to the img (check the design pattern).

  Keep coding :D

  Ps. Please, don't forget to upvote any comments on here that you find helpful.

  Greetings :D

  2

  ---

  

  Vaishnav•185

  @vaishnavmePosted almost 4 years ago

  @SzymonRojek Thank you so much for your feedback. Thanks for suggesting BEM naming convention , I didn't know about that, I'll learn about it and start using it.😄

  1

  

  Szymon Rojek•4,540

  @SzymonRojekPosted almost 4 years ago

  @vaishnavme

  I am happy that you like it.

  check this article => BEM For Beginners: Why You Need BEM

  Thanks for upvoting my comments :D

  0