Interactive rating component with html,css,js

Community feedback

• 

  Shawn Lee•560

  @OGShawnLeePosted over 2 years ago

  Hey. Good job!

  1. Firstly. If you are using VS Code I recommend installing Prettier to keep your code looking nice and tidy.
  2. The HTML seems fine some improvements would be not wrapping the rating result inside of a <p/> tag. I think a <div/> and a <span/> inside it would be better.
  3. CSS. Well I think you could have used CSS variables and relative units like rem instead of px. I don't remember when was the last time I used vanilla CSS so I cannot give you more feedback about it :'(
  4. About the JavaScript you should have used a loop to add an event listener to each button instead of hard coding each one. If you had 100 buttons that would not be easy to do. You could have got all the buttons with a querySelectorAll and loop over them with a forEach loop. Get the element and the index, add an eventListener and handle the click event with a function that uses the index as the rating. Maybe I could help with a pull request in case you get stuck.

  Hope my feedback was useful. Overall good job mate :) Have a great day/night.

  Marked as helpful

  2
• 

  CyrusKabir•1,885

  @CyrusKabirPosted over 2 years ago

  Hello my dear friend ♥ you did very well on this challenge ☻ and here some tips for your code and component :

  • it's always good to have some transition on active states (hover, focus ,...) and you can check this link about transition if you want transitions
  • in js, you add event listeners for each button separately but you can select all buttons with same class name (for example .rating-button) and iterate over them and add event listener to them and for the value you can use event object here the link about event object Event and with this you can save a lot of time maybe for example later you want add more rating buttons (1 to 10) and with your way you should add more specific button class and then select each of them and add eventListener(),

  Marked as helpful

  2
• 

  iago•400

  @iagohenrique2009Posted over 2 years ago

  Hello!! thanks for the help @OGShawnLee and @CyrusKabir

  i made a few changes like adding some transition and tried to make a loop for the btns

  PS: i tried to change px to rem but i think rem is hard for me

  I really, really appreciate yours help :)

  1

  ---

  

  CyrusKabir•1,885

  @CyrusKabirPosted over 2 years ago

  @iagohenrique2009 rem it's one of the relative units and it's not something hard don't worry about it you can watch some videos on youtube or you can check this links rem in css, css units

  Marked as helpful

  2

  

  Shawn Lee•560

  @OGShawnLeePosted over 2 years ago

  @iagohenrique2009 Glad to help mate. Well rem is based on the font size of your document (the html element I believe) which by default is 16px. So if you give something 2rem that means it is 32px and so on. Tailwind CSS really helped me use more rem because its design system is very good and it is mostly based on rem.

  I saw you added a good ol' trusty loop, it is kinda hard to read but you got the idea and it does the job just fine.

  Rem is cool because when you change the root font-size everything scales accordingly and you don't have to do extra work.

  Don't worry mate if you need help you can count on me :)

  Marked as helpful

  2