Social Proof Section - Mobile first, Responsive, flexbox & grid.

Community feedback

• 

  mycrochip•460

  @mycrochipPosted over 2 years ago

  Hi again George,

  It's nice seeing that you've also completed this challenge. You've put forward a good implementation of the solution to the project. There is only so much room for improvement.

  I believe you will put to good use my suggestions, that is why I put the following forward:

  • Your project shot would be closer to the "comparison image" if you inspect your page and set device-width to the required pixel (375 or 1440) as you build. (It's already so commendable that you are concerned about pixel-perfection. I too feel strongly about that).

  One thing I do as well but do not strongly recommend is to set body height property to 100vh (i.e full viewport height) for only single non-scrolling pages (or landing pages). From there I can easily see where I need to stretch things out using margins and paddings.

  • I understand your reason for removing the attribution, but it is not necessary to do so. If you feel it would interfere with your layout comparison, you could set the attribute background-color and position: absolute; bottom: 0 (after body{position: relative}) to remove the attribution from the flow.

  • With regards to your code clean-up concern, you could start by editing the provided HTML boilerplate before touching a single line of CSS. Move all internal styles to an external stylesheet). I also noticed that you linked to fonts in your HTML as well as imported fonts into your stylesheet. You should only use stylesheets to import your fonts unless you need to delay the loading or add other functionality to the fonts like 'preconnect')

  • Of all the above, the practice which would most significantly reduce your HTML code and your CSS (most efficiently) is to use components. The following is a copy from the README.md file attached with my solution at GitHub

  <section class="ratings">
    <article class="rating rating--1">
      Rated 5 Stars in Reviews
    </article>
  </section>
  <section class="cards">
    <article class="card card--colton">
      <h2 class="card__name" >Colton Smith</h2>
      <p class="card__status" >Verified Buyer</p>
      <p class="card__text">"We needed the same printed design as the one we had ordered a week prior. 
        Not only did they find the original order, but we also received it in time. 
        Excellent!"</p>
    </article>
  </section>
  

  The following code contains the css used in styling a rating component. I was particularly proud of the pseudo-element section where I used the background-repeat attribute to get duplicate stars and controlled the number of stars using width.

  .ratings,
  .cards {
      width: 100%;
      display: flex;
      flex-direction: column;
      margin-bottom: 3rem;
      gap: 1rem;
  }
  
  .rating {
      color: var(--clr-primary);
      font-size: 1rem;
      font-weight: 700;
      background-color: var(--clr-pale);
      border-radius: 0.5rem;
      padding-block: 3rem 1rem;
      width: 100%;
      margin: auto;
      position: relative;
  }
  
  .rating::after {
      content: "";
      width: 100px;
      height: 20px;
      position: absolute;
      background-image: url(images/icon-star.svg);
      background-repeat: space;
      inset: 1rem 0;
      margin: 0 auto;
  }
  

  • The BEM CSS notation also plays a key role in my use of components. I also built only one rating and one card (testimonial) section fully before copy/pasting and adding only single-unique classes for each replica as in the above.

  I hope this review proves helpful and I look forward to seeing your future solutions.

  EXTRA: Just as you pay good attention to the pixel-perfection, it would be nice to consider fulfilling the accessibility issues showing up in your solutions. This helped me, and in your case, it could also inspire you to use semantic HTML.

  If you feel the need, I recommend that you visit my solution as well.

  Best of luck to you and happy coding!

  Marked as helpful

  2

  ---

  

  George Stawowczyk•210

  @Jorgus1710Posted over 2 years ago

  @mycrochip Thanks so much for the well thought out answer! I will most certainly be looking into your advice, again very much appreciated!

  1
• 

  mycrochip•460

  @mycrochipPosted over 2 years ago

  👀 Did @mattstuddert just upvote my comment!

  🤩

  2

  ---

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted over 2 years ago

  @mycrochip, haha, I did indeed! 😆

  1