Social Proof Section (responsiveness to come)

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, nice work on this one. Like what you said, there is no mobile view for the site at the moment. On the other hand, desktop layout looks fine I think, just needed for the h1 to be bit bigger, the text below it could use a more darker color, each person's image could be resize down and the font-family is not properly applied.

  David Turner already gave great feedback on this one, just going to add some as well:

  • It would be really great to create a main tag that will wrap the 3 div so that they will be placed inside a landmark element.
  • Avoid using id to target and style an element since it is a bad practice due to css specificity. Instead, just use class to target element.
  • On those review section, you shouldn't have done p tag with lots of the img tag inside it. For this, I would recommend using those star images as value for the background property. background can take multiple images and this way, you won't have to create multiple img tags. But if you insist on doing so, maybe use something like this:

  <div>
    <img src="" alt="">
    <p></p>
  </div>
  

  • When using img tag, do not forget to add the alt attribute, whether the value is empty or not. Because by not including it, screen-reader will instead read the source path of the image which you don't want. So always include it.
  • For each of the testimonial card, you can use this markup below so that it will be easier for screen-reader users to traverse it and know the content via blockquote:

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

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

  • When using img tag, you don't need to add words that relates to "graphic" such as "image" and others, since img is already an image so no need to describe it as one.
  • Lastly, just in general. Whenever you submit a solution, always make sure that you completed it, meaning it will contain the mobile state and maybe making the layout as close like the design.

  Aside from those, great job again on this one.

  0
• 

  P

  David Turner•4,150

  @brodiewebdtPosted almost 3 years ago

  Add this code to replace yours on the body tag: background-position: left top, bottom right;

  Also put your stylesheet below any Google Fonts declaration, or the fonts won't work. All of your images should have empty ALT tags. They don't need text because they are display only graphics.

  Wrap your container div in a Main tag and it will help fix some of the the accessibility warnings.

  Download AXE DevTools and you can clear accessibility warnings while you code. https://www.deque.com/axe/devtools/

  Hope this helps.

  0