Responsive social proof section using Flexbox, media queries

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, really nice work on this one. Desktop layout looks great, the site is responsive and the mobile state looks great as well. Neat!

  Some suggestions on the site would be:

  • For this one, you should have used the main to wrap all the main-content of the site since this is the whole main-content. footer is not needed in here as well so you can remove those but leave the footer for the .attribution and use this markup:

  <main />
  <footer /> # must be outside the main,  attribution in here
  

  • If you zoom out from the layout, you will notice that the content are being stretched, to prevent this, use a max-width on the body tag so that the layout will prevent the content from being stretched for wide screen.
  • article and aside is not needed in here as well. You use article when you want something, a component to be redistributable for other parts of the site since it is independent content. aside on the other hand, I don't have much experience about aside to be honest, but I think replacing it with just div would be fine:>>
  • Those star-icons are only decorative. Decorative images should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if you are using svg instead of img tag.
  • Only use descriptive alt on images that are meaningful and adds content to the site otherwise hide the image for screen-reader users.
  • Also, another approach for those stars would be using them as value for the background property for each .stars selector. The background property can accept images as many as you want, have a search about background.
  • When wrapping up a text-content, make sure that it is inside a meaningful element like p tag or heading tag and not using like div, span to wrap the text.
  • For each of the testimonial card, you can use this markup so that it will be easier for users to navigate through it:

  <figure>
    <img src="" alt={person name}>
    <blockquote>
      <p> {qoute in here}</p>
    </blockquote>
    <figcaption>
      person name
      <p>
        person role
      </p>
    </figcaption>
  </figure>
  

  Though you would need to use display: grid on this one so that you could place each item of the figure properly like on the design.

  • Also, when using img tag, you don't need to add words that relates to "graphic" such as "logo" and others, since img is already an image so no need to describe it as one.

  Aside from those, great job again on this one.

  Marked as helpful

  1
• 

  Martin Eliáš•890

  @martinelias1312Posted almost 3 years ago

  I am really grateful that You took the time for inspecting my code and to gave me a feedback. I already fixed some things like You said, and i learned new things.

  Thanks!

  1