FunkyCreep• 130
@francoisbillet
Posted
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 usingvar(--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