Interactive rating component

Community feedback

• 

  Icaro Mendes•240

  @icaroMendes777Posted about 2 years ago

  Nice,

  just noticed 2 things:

  -the "how did we do" text is too small. Put it between <h2> tags to match the design

  -And one practical mistake for a real project: if you click the submit button without giving any rating from one to five, it is submitting a rating five by default. If the user didn't give a 5, it is weird to rate five. It is just to put some validation before submitting, so that the user doesnt submit it without giving it a rating before.

  Frequently you as a programmer gotta deduce these requirements even when it is not mentioned in the project. But this is an extra, the project it is mostly very good!

  Marked as helpful

  0

  ---

  

  Michal Bednář•140

  @MichalBednarPosted about 2 years ago

  @icaroMendes777 Thank you so much for suggestion, I will fix the "how did we do" text. But since I'm not good with JS, the rating improvement will have to wait.

  0

  

  Icaro Mendes•240

  @icaroMendes777Posted about 2 years ago

  @MichalBednar dont worry, the goal of this community is to receive advice,

  for the validation is actually easy:

  declare a global variable in the begining of the javascript, lets say "var flag=false" and when a rating is given you set "flag=true". So, when the submit function is called, you check: " if (flag==false) return" This means that if no ratting was given "flag" remain false and the submit function will not be completed. If at any moment a rating was given, flag will have the value true, and the submit can skip that line and continue.

  I was looking at your code and one big improvement can be done. All those 5 rating functions can be made into one.

  you make a general function:

  function giveRating(rat){

  ///here you select the element related to that rating and make the changes
  

  const rating = document.getElementById("rating"+rat);

  ///etc...

  }

  and in each button you put the property

  <button id="rating5" onclick="rate(5)">5</button>

  this will call the function 'rate' and pass the argument (5) to the function when the button 5 is clicked.

  Repeat yourself the minimum, this is what differentiates good from not so good code.

  but basically your code works, the job is done, this is just a tip for improving your skills.

  1

  

  Michal Bednář•140

  @MichalBednarPosted about 2 years ago

  @icaroMendes777 Thank you, I will try to do it

  0
• Account deleted

  Hello Michal Bednář 👋

  In Javascript you use innerHTML property to change content of rating span instead of using textContent which is more appropriate. Problem with innerHTML is that you can set arbitrarily html code inside of element. It's overkill for most usages and is a sink for XSS vulnerabilities

  What an XSS vulnerability

  XSS vulnerability is a client side security vulnerability where an attacker can make victim execute arbitrary html/javascript code by using malicious variables. You can learn a lot about and how to fix them properly on :

  • XSS Prevention

  Fix

  Replace every occurence of innerHTML with textContent and use innerHTML at least ressort

  Otherwise, congratulation for your achievement 🎉

  Marked as helpful

  0
• 

  Fazza Razaq Amiarso•2,340

  @fazzaamiarsoPosted about 2 years ago

  Hi Michal, Nice solution!

  I have some tips to further improve your code.

  • Button element actually have value property you can set. Here is my html.

  <ul id="rating-options">
        <li><button class="btn1 option" value="1">1</button></li>
        <li><button class="btn2 option" value="2">2</button></li>
        <li><button class="btn3 option" value="3">3</button></li>
        <li><button class="btn4 option" value="4">4</button></li>
        <li><button class="btn5 option" value="5">5</button></li>
      </ul>
  

  • You can add the listener to the ratings button container by using "Event Delegation" technique to simplify your code.

  const ratings = document.getElementById("rating-options");
  ratings.addEventListener("click", e => {
      e.preventDefault();
      const rateButton = e.target
      if(!rateButton.classList.contains("option")) return;
      setRating(rateButton.value);
  })
  
  function setRating(selectedValue){
      rating.innerText = selectedValue;
  }
  

  Event delegation: https://javascript.info/event-delegation

  I hope it helps! Good Luck!

  Marked as helpful

  0

  ---

  

  Michal Bednář•140

  @MichalBednarPosted about 2 years ago

  @fazzaamiarso Thank you for your help, I have poor JavaScript knowledge, so I don't understand the JS code you suggested clearly, but once I get better, I will try to implement it.

  0