Interactive rating component

Community feedback

• 

  Tobias Fuchs•40

  @ToFu96Posted over 2 years ago

  Hey, I recently did this challenge too and at first glance your solution seems very good to me. I like how you implemented the JavaScript. I'll try to remember some of your tricks, because they are cleaner than what my JavaScript code looks like (I didn't publish my code).

  Some comments (note that some are a bit nitpicky):

  • I might be wrong, but I think the submit button should change on hover. Otherwise the button doesn't feel as clickable.
  • Maybe consider renaming some classes. For example, "number" doesn't say much outside of context. This isn't a problem in a small project, but in bigger ones it increases the time to understand the code, especially for someone who never saw it before. Maybe something like "rating-button" is more describing? If you rename, make sure it's renamed in the JavaScript file as well.
  • You may want to read a bit about "px", "em" and "rem". From my understanding, setting font-size via pixels is bad practice regarding accessibility. For example, if someone (with bad eyesight) increases their browser's font size, text whose size was set via pixels won't be affected by that. I wouldn't get too worried about all this right now, but it's definitely worth researching what "em" and "rem" do and when they should be used.
  • If you are up for an additional challenge, try to change the submit button in a way that it only becomes clickable once you selected a rating option. Otherwise, make it unclickable and with lower opacity. You could create a new git branch to implement that feature and (if you are happy with your implementation) merge it with your main branch once you are done.

  That's all I have to say. Cheers!

  Marked as helpful

  1

  ---

  

  Kirsten•60

  @kmalcabaPosted over 2 years ago

  @ToFu96 Hi there! Thanks for the info, you have some good points there.

  I initially wanted to get this challenge done quickly just as a warm up so I might not have followed some best practices, but it makes sense to be consistent with stuff like this instead of just getting things done and sacrificing readability/accessibility along the way.

  Thank you so much, I'll keep your feedback in mind! <3

  1
• 

  Abdul•8,540

  @SamadeenPosted over 2 years ago

  Hey!! Cheers 🥂 on completing this challenge.. .

  Here are my suggestions..

  • You should use <main class="rate-box"> instead of <div class="rate-box">.
  • Go down orderly when you are using the headings h1 down to h2 down to h3 and so on.

  This should fix most of your accessibility issues

  . Regardless you did amazing... hope you find this useful... Happy coding!!!

  Marked as helpful

  1

  ---

  

  Kirsten•60

  @kmalcabaPosted over 2 years ago

  @Samadeen Thanks for the feedback!

  0