Responsive design done with Flexbox and SASS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Layout in desktop looks fine, the elements are just spaced-out more than the original. It is somehow responsive and the mobile state looks great.

  Some suggestions would be:

  • It would be great to have a base styling with these props:

  html {
    box-sizing: border-box;
    font-size: 100%;
  }
  *,
  *::before,
  *::after {
    box-sizing: inherit;
  }
  

  Using this, element's will be easier to handle in terms of sizes since padding and margin won't add any extra size to it, compared to when it is not using that property.

  • You don't need to use height: 800px on the body you could just remove that and let the element inside it define the size of the body tag.
  • Each of those stars are only decoration. Decorative image must be hidden at all times by using alt="" and extra aria-hidden="true" attribute on the img tag.
  • Also, instead of using img for each stars, you could have use those inside a background property. Have a search about adding multiple images in background property. This will save you up some htmlt element that is not really needed.
  • When wrapping a text-content do not just use div to wrap it, use meaningful element like a p tag if it just a regular text or heading tag if it is an heading.
  • Person's img should be using the person's name as the alt like alt="Colton Smith. Lose the word img. When using alt attribute, avoid using words that relates to "graphic" such as "Img or image" and others. An img is already an image/graphic so no need to describe it as one.
  • The person's name as well their position could just use a div and not ul since it is not really needed. Also the person's name should be a heading tag like h2 since they are the focal point of the section.

  Aside from those, great work again on this one.

  Marked as helpful

  2