@abrahamemamani
Posted
Hi, Charles! Well done! There are a couple of things you can improve.
- In the rating.tsx file you are repeating a lot of code. You should declare an array of objects and use the Choice component only once inside a map loop.
- If you are following the BEM rules, activeChoice should be choice--active.
- Do not use HTML tags in styles, it is a very bad practice. You should only use them in the generic file.
- When you are using sass/scss, @media should be declared inside the selector you want to modify.
Hope you find this helpful. Happy coding!
Marked as helpful
@charlesmiller0412
Posted
@abrahamemamani great advice! I feel like taking breaks can sometimes make me lose progress.
- Looking back, I don't know why I didn't think of mapping an array of numbers, but thank you for making me use my brain to create cleaner code!
- I usually use camelCase when being used in a boolean condition (such as a variable), but can also see why that could be confusing to others reviewing my code. I corrected this and will remember in the future.
- The only HTML tag I can find as a style would be rating__submit..if this is not the one you speak of, please let me know as I'd like to understand!
- I actually didn't know that about @media in scss and I can see how much cleaner this looks.
I'm pushing the changes you recommended. Honestly, this is the most helpful review I've received. Keep it up! There's always room for us to improve!
@abrahamemamani
Posted
@charlesmiller0412 Hi! In the third point I was referring to the selectors. For example, in the __rating.scss file you used html tags (picture, img, h1, p, span). Those selectors should only contain class names (&__picture, &__title, etc). In large projects it is a big problem.
Marked as helpful
@charlesmiller0412
Posted
@abrahamemamani Understood! Already updated and will keep this in mind for my future projects.
