Are there better alternatives to my current usage of onclick?

user_10906514 0
Viewed 84

I am putting together a form and some content will be conditional based on a user's radio button selections. I've got code that works as I intend (hide or show followup content based on selection). However, I can only do it using onclick within the html (i.e. "onclick=checkRadioAnswer()"). See snippets below for example.

I would prefer to keep it all in one JS file and not use onclick in the HTML. I figured addEventListener() would be the way to go but I cannot get it to work for the life of me.

Any suggestions on how I might rework this code so that it will act the same as my current setup but without using onclick in the html? I also need to keep this plain JS. Thanks

window.onload = function setDefaultState() {
  checkRadioAnswer(); 
};

const changingContent = document.getElementById("changing-content");

function checkRadioAnswer() {
  var radios = document.getElementsByName("genericRadio")
  for (var i = 0, length = radios.length; i < length; i++) {
      if (radios[i].checked) {
        if (radios[i].value == "1") {
          changingContent.style.display = "none";
        }
        if (radios[i].value == "2") {
          changingContent.style.display = "block";
        }
        if (radios[i].value == "3") {
          changingContent.style.display = "none";
        }
        if (radios[i].value == "4") {
          changingContent.style.display = "block";
        }
      }
    }
};
<fieldset class="fieldset-radio">
  <legend>
    <h1 class="question-primary">The question goes here</h1>
  </legend>
  <div class="field-radioButton">
    <label for="answer1">
      <input id="answer1"
        type="radio"
        name="genericRadio"
        value="1"
        checked="checked"
        onclick="checkRadioAnswer()">
      <span>1st answer text</span>
    </label>
  </div>
  <div class="field-radioButton">
    <label for="answer2">
      <input id="answer2"
        type="radio"
        name="genericRadio"
        value="2"
        onclick="checkRadioAnswer()">
      <span>2nd answer text</span>
    </label>
  </div>
  <div class="field-radioButton">
    <label for="answer3">
      <input id="answer3"
        type="radio"
        name="genericRadio"
        value="3"
        onclick="checkRadioAnswer()">
      <span>3rd answer text</span>
    </label>
  </div>
  <div class="field-radioButton">
    <label for="answer4">
      <input id="answer4"
        type="radio"
        name="genericRadio"
        value="4"
        onclick="checkRadioAnswer()">
      <span>4th answer text</span>
    </label>
  </div>
</fieldset>

<div id="changing-content">
  <p>Content depending on answer goes here</p>
</div>

html javascript
2 Answers

You might consider using event delegation instead - listen for change events on the container of the radio buttons, then use an object indexed by radio button value to determine what display style to set:

const changingContent = document.getElementById("changing-content");
const container = document.querySelector('.fieldset-radio');
container.addEventListener('change', checkRadioAnswer);

const displayByValue = {
  1: 'none',
  2: 'block',
  3: 'none',
  4: 'block'
}
function checkRadioAnswer() {
  const checkedRadio = container.querySelector('input[type="radio"]:checked');
  changingContent.style.display = displayByValue[checkedRadio.value];
}

checkRadioAnswer();
<fieldset class="fieldset-radio">
  <legend>
    <h1 class="question-primary">The question goes here</h1>
  </legend>
  <div class="field-radioButton">
    <label for="answer1">
      <input id="answer1"
        type="radio"
        name="genericRadio"
        value="1"
        checked="checked">
      <span>1st answer text</span>
    </label>
  </div>
  <div class="field-radioButton">
    <label for="answer2">
      <input id="answer2"
        type="radio"
        name="genericRadio"
        value="2">
      <span>2nd answer text</span>
    </label>
  </div>
  <div class="field-radioButton">
    <label for="answer3">
      <input id="answer3"
        type="radio"
        name="genericRadio"
        value="3">
      <span>3rd answer text</span>
    </label>
  </div>
  <div class="field-radioButton">
    <label for="answer4">
      <input id="answer4"
        type="radio"
        name="genericRadio"
        value="4">
      <span>4th answer text</span>
    </label>
  </div>
</fieldset>

<div id="changing-content">
  <p>Content depending on answer goes here</p>
</div>

user_95152070

Welcome to SO.

I would recommend adding a class to each of your input tags, and then adding the event listener:

var inputs = document.querySelectorAll('.inputClass');

for(var i = 0; i < inputs.length; i++) {
    inputs[i].addEventListener('click', checkRadioAnswer, false);
}

Or something to that effect.

Edit: Added for loop in place of forEach, since NodeList.prototype.forEach is only supported in newer browsers (Thanks @CertainPerformance)

user_69466250
Related