Design comparison
SolutionDesign
Solution retrospective
All feedback is greatly appreciated - thanks!
Community feedback
- @SutonTochPosted over 1 year ago
Congrats on completing the challenge!
I really like:
- Your GitHub. Your README shows that you have taken extra time to make it look nice.
- Your JavaScript. The Solution with the event listener on the rating-container instead of on each button in conjunction with
e.target.closest(".rating-button")
looks really elegant. Good Job!
Some notes on CSS:
- You declare font-family in the body twice
- If width and height are to be absolute, I'd recommend using pixels instead of rem. Personally, I have a better understanding of what 51px is, instead of 3.2rem. But that might differ from person to person. If it's unclear, rem looks at the root font-size (usually 16px) and is therefore the more absolute/static counterpart to em which looks at the font-size of the parent or same element (depending on if you use it for font-size or not).
- Nitpick:
font-size: 1rem
andline-height: 1
doesn't do anything unless you overwrite something. I'd recommend checking your CSS in the Browser Dev-Tools for unnecessary code. Less code → more readable.
Sadly, there is one fatal flaw:
- You can submit without choosing a rating. Easy to fix with a conditional and judging by your JS I'm pretty sure you don't need a hint.
Thanks to your contribution, I learned about the
defer
attribute of theMarked as helpful0
Please log in to post a comment
Log in with GitHubJoin 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