Results summary component in React.js

Community feedback

• 

  FunkyCreep•130

  @francoisbilletPosted over 1 year ago

  Hello, It looks good, congrats !

  A few feedbacks:

  • Use semantic HTML as much as possible, you have a few warnings in your accessibility report about this. For instance in your App.js, you should put a <main> and not a div.
  • In your Results.jsx file your images have an alt property value of "icon" for every single icon. This is not very useful for a person who can't see, who's using a screen reader. Be more explicit about what represents each icon.
  • To get used to building React apps with components, I'd probably have created 2 components: one <Result /> and one <Summary />. I'd also have created a component <CategorySummary /> which would correspond to this bit of code:

  <li key={index} className={styles.category} style={{ color: item.color, background: item.background }}>
         <img src={item.icon} alt="icon" className={styles.images} />
          {item.category}
         <div className={styles.scores}>
              <p className={styles.score}>{item.score}</p> <span>  / 100</span>
         </div>
  </li>
  

  You could then pass props to this components for all the info it has to display. This is a very small project so you don't really need to create smaller components like this but as a project scales it is important to do this to help readability and maintainability.

  About CSS:

  • There is a lot of duplicate values. Imagine if you want to change the general font-size of 18px to 16px. You'd have to change it everywhere. Instead you can declare custom properties in the :root selector (html tag) and use them using var(--my-custom-property). you don't need SCSS to have variables.
  • You should use rem units for font-size (also for margin and padding). Check out the link to know why.

  Anyway, good job ! Keep up the good work :)

  Marked as helpful

  1