Is there a cleaner and less messy way to do a on click to get a value of a radio button?

Question

<div class="stars">
  <form action="">
    <input class="star star-5" id="star-5" type="radio" name="star" onclick="test(5);"/>
    <label class="star star-5" for="star-5"></label>
    <input class="star star-4" id="star-4" type="radio" name="star" onclick="test(4);"/>
    <label class="star star-4" for="star-4"></label>
    <input class="star star-3" id="star-3" type="radio" name="star" onclick="test(3);"/>
    <label class="star star-3" for="star-3"></label>
    <input class="star star-2" id="star-2" type="radio" name="star" onclick="test(2);"/>
    <label class="star star-2" for="star-2"></label>
    <input class="star star-1" id="star-1" type="radio" name="star" onclick="test(1);"/>
    <label class="star star-1" for="star-1"></label>
  </form>
</div>

<script> 
      
    function test(value)
    {
          alert(value);
    }
        
</script>

The above is what i have right now which works fine but I don't like how it has the onclick method on the input is there i cleaner and easier and better way to do this?

What do you mean by cleaner, easier, or better in *objective* terms? — TylerH, Feb 19 '21 at 07:54

score 0 · Answer 1 · answered Feb 18 '21 at 23:38

0

Try something like onclick="test(this.value);" but radio input required value attribute

answered Feb 18 '21 at 23:38

ExploitFate

595
2
9

KevBot · Answer 2 · 2021-02-19T00:01:56.187

Event delegation

Add the listener to a parent element, check to see if a radio button changed.

Events on the DOM bubble up to their parents (unless otherwise specified in custom events or delegation is prevented) until it reaches the top level element. This means you can watch for events on a single element for potentially many elements. The downside of delegation is that you have to check for the element type you are expecting.

This will also allow you to capture the use-case on any dynamically added inputs as well.

const form = document.querySelector('form');
form.addEventListener('change', ({target}) => {
    if (target.matches('input[type=radio]')) {
        console.log(target.value);
    }
});

<div class="stars">
  <form action="">
    <input class="star star-5" id="star-5" type="radio" name="star" value="5" />
    <label class="star star-5" for="star-5"></label>
    <input class="star star-4" id="star-4" type="radio" name="star" value="4" />
    <label class="star star-4" for="star-4"></label>
    <input class="star star-3" id="star-3" type="radio" name="star" value="3" />
    <label class="star star-3" for="star-3"></label>
    <input class="star star-2" id="star-2" type="radio" name="star" value="2" />
    <label class="star star-2" for="star-2"></label>
    <input class="star star-1" id="star-1" type="radio" name="star" value="1" />
    <label class="star star-1" for="star-1"></label>
  </form>
</div>

Good answer, but I think Spectric's is more on point, his will still function as expected if additional radio groups are added to the form. — TCooper, Feb 18 '21 at 23:44
@TCooper Check your initial comment here as to future readers it might suggest it still holds true. This is now by far the most concise and complete answer. — connexo, Feb 18 '21 at 23:59

Spectric · Answer 3 · 2021-02-19T03:17:58.580

Use an event listener.

I've modified the test function to log instead of alert thanks to ths's excellent suggestion.

In vanilla JS:

var stars = document.querySelector('.stars form');
stars.addEventListener("click", function(event) {
  var clicked = event.target || event.srcElement;
  var num = clicked.getAttribute("id").substring(5);
  test(num);
});

function test(value) {
  console.log(value);
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div class="stars">
  <form action="">
    <input class="star star-5" id="star-5" type="radio" name="star" />
    <label class="star star-5" for="star-5"></label>
    <input class="star star-4" id="star-4" type="radio" name="star" />
    <label class="star star-4" for="star-4"></label>
    <input class="star star-3" id="star-3" type="radio" name="star" />
    <label class="star star-3" for="star-3"></label>
    <input class="star star-2" id="star-2" type="radio" name="star" />
    <label class="star star-2" for="star-2"></label>
    <input class="star star-1" id="star-1" type="radio" name="star" />
    <label class="star star-1" for="star-1"></label>
  </form>
</div>

In jQuery:

$('.star').on("click", function() {
  test($(this).attr('id').substring(5));
});
function test(value) {
  console.log(value);
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div class="stars">
  <form action="">
    <input class="star star-5" id="star-5" type="radio" name="star" />
    <label class="star star-5" for="star-5"></label>
    <input class="star star-4" id="star-4" type="radio" name="star" />
    <label class="star star-4" for="star-4"></label>
    <input class="star star-3" id="star-3" type="radio" name="star" />
    <label class="star star-3" for="star-3"></label>
    <input class="star star-2" id="star-2" type="radio" name="star" />
    <label class="star star-2" for="star-2"></label>
    <input class="star star-1" id="star-1" type="radio" name="star" />
    <label class="star star-1" for="star-1"></label>
  </form>
</div>

Jon P recommends you to use a value attribute so when the convention for the id attribute changes, the functionality is preserved. This can be implemented like so:

var stars = document.querySelector('.stars form');
stars.addEventListener("click", function(event) {
  var clicked = event.target || event.srcElement;
  var num = clicked.getAttribute("value");
  test(num);
});

function test(value) {
  console.log(value);
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div class="stars">
  <form action="">
    <input class="star star-5" id="star-5" type="radio" name="star" value="5"/>
    <label class="star star-5" for="star-5"></label>
    <input class="star star-4" id="star-4" type="radio" name="star" value="4"/>
    <label class="star star-4" for="star-4"></label>
    <input class="star star-3" id="star-3" type="radio" name="star" value="3"/>
    <label class="star star-3" for="star-3"></label>
    <input class="star star-2" id="star-2" type="radio" name="star" value="2"/>
    <label class="star star-2" for="star-2"></label>
    <input class="star star-1" id="star-1" type="radio" name="star" value="1"/>
    <label class="star star-1" for="star-1"></label>
  </form>
</div>

Try to avoid `alert` method in the answers/questions due to its poor UX, consider using `console.log` instead. — ThS, Feb 18 '21 at 23:39
@ths Thanks for your feedback. I'll incorporate it if I ever feel the need to. — Spectric, Feb 18 '21 at 23:43
Your answer, while certainly helpful, fails to mention and explain the concept of delegate listeners (which your answer uses); instead it wrongfully claims to *Attach an event listener to each element.* Also, your delegate listener fails to make sure the clicked element is actually one you expect. Simply click to the right of the radio buttons to see your code fail for this reason. — connexo, Feb 18 '21 at 23:45
Even though you corrected *Attach an event listener to each element.* you still do not explain the concept of delegate listeners (which OP almost certainly isn't aware of). Remember this requires to understand event lifecycles and bubbling. — connexo, Feb 18 '21 at 23:49
One further improvement would be to add the value attribute to the radio buttons, then there is no need to for string parsing of the ID. If for what ever reason the name convention for the ID's change, the javascript remains untouched. — Jon P, Feb 18 '21 at 23:51
@connexo sorry, deleted my last comment - I was only looking at the jquery, not the vanilla js which is clearly selecting the parent. — TCooper, Feb 18 '21 at 23:54

score -1 · Answer 4 · edited Feb 19 '21 at 00:04

In my opinion, Spectric's answer is the best because it allows to keep a very good readability of the code.

But to express another possibility, you could add a name to the form and create the content dynamically. This way there would not be as much repetition and if there is a need to increase the number of radioboxes in the future you will only have to modify the conditions of the for loop.

var formSelector = document.getElementsByName('form_name')[0];
var radiobox, label;

for(var i = 1; i <= 5; i++) {
   radiobox = document.createElement("input");
   radiobox.setAttribute("class", "star star-" + i);
   radiobox.setAttribute("id", "star-" + i);
   radiobox.setAttribute("type", "radio");
   radiobox.setAttribute("name", "star");
   radiobox.setAttribute("onclick", "test(" + i + ")");

   label = document.createElement("label");
   label.setAttribute("class", "star star-" + i);
   label.setAttribute("for", "star-" + i);

   formSelector.appendChild(radiobox);
   formSelector.appendChild(label);
}

function test(value) {
   alert(value);
   // or console.log(value) if you don't want a popup everytime you click on a radiobox
 }

<div class="stars">
 <form action="" name="form_name"></form>
</div>

This answer is far out of scope of the question. Also, consider teaching yourself about element properties. There's more in the DOM API than getAttribute and setAttribute. — connexo, Feb 19 '21 at 00:02
Well i'm not into HTML/JS but I saw an opportunity to help even though I know my code is probably not optimized. Thanks for the advice and if someone wants to reuse my code and optimize it he can already take inspiration from this: https://stackoverflow.com/a/12274782/12315190 — Tanguy Lhinares, Feb 19 '21 at 00:07

Is there a cleaner and less messy way to do a on click to get a value of a radio button?

4 Answers4

Event delegation

Use an event listener.

In vanilla JS:

In jQuery: