Result Summary Component - React, Typescript, Vite, SCSS

Solution retrospective
Hello fellow coders, On this project, just like the other ones, I have tried something different. Even though it is a small project, I decided to take on the challenge using the best practices when it comes to SCSS. I didn't use all the features available such as layout and functions, but I have used mixins and adding breakpoints using the mixins. I will continue to work on my SCSS skills on the following projects, so if you guys have any questions or suggestions please feel free to ask or share. Feedback is always welcome.
Please log in to post a comment
Log in with GitHubCommunity feedback
- P@jairovg
Hi @newbpydev, congrats on your solution; here are some comments that might help you to improve it:
- First of all, nice use of a modularized structure of your styles, using SCSS variables, BEM and mixins.
- A couple of suggestions around this. (1) using colour variables linked to colour names, like red or yellow may take you to unnecessary refactors if the colour change; instead, try to use variables scoped to features or categories, (2) I invite you to use also CSS variables
- You're setting a fixed
font-size
of10px
to the page, so even as you're usingrem
for your font sizes, you're injecting an accessibility issue, as even if the user changes the browser font size, they will be always calculated with a base of10px
. - It might be a discussion if it's better to use
rem
or fixed units likepx
to offset values, likemargin
orpadding
; personally I think that usingrem
of these offsets is not the best option as if a user increases the browser font size, you're reducing the content space and it may get the component to rare visual states. - The button doesn't have a focus state which induces an accessibility issue. A user that uses the keyboard to navigate the page will never know when is on the button element unless it's using assistive technology.
- Take a look at devices with medium sizes, like iPad Air or Mini, with
820px
and768px
each. Even there is no design for those devices, if you take a look, the component seems to be floating. - This is your page
heading map
:
1 Your Result 2 76 2 Great 2 Summary 3 Reaction 3 Memory 3 Verbal 3 Visual
Think about a table of contents of the page and see if it's a good distribution. I'm not saying yours is good or bad, just think about it.
- There are some recurring categories in the summary section, is it a
div
the best semantic element for this?
I hope you find it useful. I'm happy to take another look at your solution if you make some other changes.
Marked as helpful - @0xabdulkhaliq
Hello there 👋. Congratulations on successfully completing the challenge! 🎉
- I have other recommendations regarding your code that I believe will be of great interest to you.
DECORATIVE SVG'S ♨️:
- The
alt
attribute is used to provide alternative text for images in HTML documents. Thealt
attribute is used by screen readers to describe the image to visually impaired users, which is essential for web accessibility.
- Now, when it comes to decorative
SVGs
, they are used purely for aesthetic purposes and do not convey any important information or functionality to the user.
- Since these images do not convey any important information or functionality, there is no need for an
alt
attribute.
- So feel free to set the
alt
attribute as""
for decorativesvg's
, becausealt=""
will be skipped by screen readers they will consider the image as decoration
Example:
<img src="images/decorative.svg" alt="">
<img src="./assets/images/icon-reaction.svg" alt="Reaction" class="summary-stat__icon"> 👇 <img src="./assets/images/icon-reaction.svg" alt="" class="summary-stat__icon">
.
I hope you find this helpful 😄 Above all, the solution you submitted is great !
Happy coding!
Marked as helpful
Join our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord