interactive-rating-component

Community feedback

• 

  Boots 😺•1,590

  @adityaphasuPosted over 1 year ago

  Hello!

  While it's okay with a list or a div with buttons...Here you can actually pretty much do it with a form as well with radio type input buttons (those circle inputs with dots inside yes those) As we are taking the rating as an input from the user a form with input makes more sense. Using input buttons will actually also cut down your javascript a bit.

  Here's a basic rundown of how we are going to do it before I explain with code snippets: Add inputs and labels instead of buttons -> Style them using CSS -> Then see how we can simply use input values to change the content of thank you page

  Okay, let's begin!

  • First, remove the buttons from the .rate class and wrap the .rate with a <form> tag and move the submit button inside the form like this:

  <form action="#">
     <div class="rate>
     </div>
  <button class="submit" id="submit" type="submit">Submit</button>
  </form>
  

  • Make sure to add a type of submit to the button as we are now inside a form.
  • Now inside the .rate div, we are going to add 5 inputs and labels. Inputs will of the type="radio" and Labels will have the values 1-5 in them like this

  <input type="radio" name="number" id="rate1" value="1">
  <label for="rate1" class="numbers">1</label>
  
  <input type="radio" name="number" id="rate2" value="2">
  <label for="rate2" class="numbers">2</label>
  
  <input type="radio" name="number" id="rate3" value="3">
  <label for="rate3" class="numbers">3</label>
          
   <input type="radio" name="number" id="rate4" value="4">
  <label for="rate4" class="numbers">4</label>
  
   <input type="radio" name="number" id="rate5" value="5">
  <label for="rate5" class="numbers">5</label>
  

  • I have used the same class .numbers on the labels as you have on the buttons currently. You might wonder why we have used the same name attribute number for all the radio buttons, it's because selecting any radio button in the group with the same names will automatically deselect any currently-selected radio button in the same name group and select the one you selected.
  • We have set id on inputs and for on labels so that we know which label belongs to which input and in CSS we are going to use this which will be a huge help!

  Now that we are done with some Html let's move on to the CSS part:

  • You already have good flex properties on the .rate you just need to add padding: 2rem 0 to give the container top and bottom padding. (We give the padding here because we are going to remove the margin in the .numbers CSS and add some new CSS to it)
  • Firstly we will hide the inputs by using display: none like this:

  .rate input {
  display: none;
  }
  

  • You might ask if we just set the display to none how are going to select inputs, remember we set id and for for our inputs and labels? these connect them to each other and when you say click on the label the input gets selected.
  • Now change the button selector to .numbers selector (since we are going to style the labels and those have a class of .numbers) while keeping those styles. We are just going to add some few more CSS properties to and it should look like this :

  .numbers {
      border-radius: 50%;
      width: 3rem;
      height: 3rem;
      background-color: hsl(213, 19%, 21%);
      color: hsl(217, 12%, 63%);
      display: flex;                                  //These are the new ones that I have added
      justify-content: center;             // don't forget to remove the margin property 
      align-items: center;
  }
  

  • I have added flex properties so that the numbers inside the label will get centered inside it.
  • Now onto the CSS part where we want the label to get the color grey when an input is selected (remember those 2 are connected even if the input is not displayed it still gets selected if you click on the label)

  .rate  input:checked + label {
     background-color: hsl(216, 12%, 54%);  // medium grey
     color: hsl(0, 0%, 100%);  // white
  }
  

  • Let me quickly explain what we did here, we are saying inside the .rate div the input which is checked (input:checked) +(it's sibling) if it's a label then we add ab bg color and set text to white. basically, it means the label of the selected input.

  Now that the CSS part is finished let's move to javascript since you have all the elements selected already in this regard you don't have to target more (I have modified my feedback according to the classes you have used)

  • All of the stuff will be handled by your button only! Since we get that added functionality of the input buttons to get selected on click from the start we don't need .forEach to listen for clicks individually now! That means less javascript code!
  • Remember we have given those values to each of our inputs? we are going to get the one for the selected one and just display the value of it inside the result like this:

  submit.addEventListener("click", (e) => {
      e.preventDefault();
      let selectedNumber = document.querySelector('input[name="number"]:checked').value;
      result.textContent = selectedRating;
  
      container.style.display = "none"
      container2.style.display = "flex"
  })
  

  • Let me explain the code I added: we pass an event (e) as a param and use e.preventDefault() so that the form when submitted by our button doesn't refresh our page.
  • We make a variable (you can call it whatever you like I have used selectedNumber) and then we use document.querySelector('input[name="number"]:checked').value to get the value of the input which got selected (for example say if we click 1st label the 1st input would be selected and its value is 1 so we will get 1 here) and then we simply set the result text content to this variable we made!
  • Now we won't even need to use .forEach and we can remove it.

  This feedback is quite long I know and you might even feel this is unnecessary but if we can cut our code and make it simple then I say it's worth a shot!

  Good luck and keep up the hard work!😊

  Marked as helpful

  0

  ---

  

  tired-herb•120

  @tired-herbPosted over 1 year ago

  @adityaphasu Thank you so much for your help! Your feedback is so helpful!

  1
• 

  Yefree Valdez•840

  @yefreescodingPosted over 1 year ago

  👋 Hey, you've done a great job with this solution. After reviewing it, I have a couple of tips for you to enhance the readability, semantics, and overall structure of your code. Additionally, I have a solution that might address the question you posed to the community.

  First tip:

  Incorporate more semantic HTML. In your component, you've extensively used <div> elements, which is generally considered a suboptimal practice:

  In your component you used a lot of <div>, that is considered a bad practice:

  ❌
  <div class="container">
      <button>
      <img src="images/icon-star.svg" alt="star">
      </button>
      <h1>How did we do?</h1>
      <p>Please let us know how we did with your support request. All feedback is appreciated 
        to help us improve our offering!</p>
      <div class="rate">
        <button class="numbers">1</button>
        <button class="numbers">2</button>
        <button class="numbers">3</button>
        <button class="numbers">4</button>
        <button class="numbers">5</button>
      </div>
        <button class="submit" id="submit">SUBMIT</button>
    </div>
  

  A better solution would be something like this:

  ✅
  <main class="wrapper">
    <div class="container">
    <div class="container_icon">
      <img src="images/icon-star.svg" alt="star icon"/>
    </div>
    <h1>How did we do?</h1>
    <p>
      Please let us know how we did with your support request. 
      All feedback is appreciated to help us improve our offering!
    </p>
    <form class="container_form">
      <fieldset class="rate">
        <label for="star1" >
          <input id="star1" type="radio" name="rate" value="1">
          1
        </label>
        <label for="star5" >
          <input id="star5" type="radio" name="rate" value="1">
          2
        </label>
        // rest of the inputs...
      </fieldset>
        <button class="submit" type="submit">SUBMIT</button>
    <form>
    </div>
  </main>
  

  Things I changed:

  • Wrapped all the component inside of a <main> container
  • The image of the star icon should be wrapped in a <div> not a <button>
  • Replaced <div class="rate"> for a <form class="rate">
  • Used <label> elements associated with each radio input for better accessibility and user experience.
  • Changed the "SUBMIT" button to a proper <button> element with the type attribute set to "submit" for proper form submission.

  Regarding to your question "Is it okay or with a list it would be better? What do you think?".

  I think there's no reason why you should use a list in this case, so you do not have to worry about it.

  I hope these tips could help you become a better front end developer!!

  Marked as helpful

  0

  ---

  

  tired-herb•120

  @tired-herbPosted over 1 year ago

  @yefreescoding Thank you so much for your feedback! I really needed this information and it is so helpful! I will definitely use it in my projects.

  1