JavaScript function that toggles element visibility according checked radio input

Question

This function has a set of radio inputs, for each radio there is a hidden text input. When a radio input is checked it shows its chained text input.

I've done it with a function constructor, and I could have done this with closures or with simpler invocation patterns. My idea is to have some robust function which I could extend.

I haven't tested browser compatibility, which is somehow important for my market (IE8 lovers).

jsFiddle

<form>
<input type="radio" name="color" value="yellow" checked /><label>Yellow</label>
<input type="radio" name="color" value="brown" /><label>Brown</label>
<input type="radio" name="color" value="orange" /><label>Orange</label>
<input type="radio" name="color" value="purple" /><label>Purple</label>

<input type="text" id="value-yellow" class="radio-condition" placeholder="Which yellow?" />
<input type="text" id="value-brown" class="radio-condition" placeholder="Which brown?" />
<input type="text" id="value-orange" class="radio-condition" placeholder="Which Orange?" />
<input type="text" id="value-purple" class="radio-condition" placeholder="Which Purple?" />

</form>

// INSTRUCTIONS
// There is a hidden text input for each radio input.
// Checked radio will show its chained text input.
// They're chained by the radio value and the text id
// the radio value makes a string that is used as the id of the text

// Constructor Function
var JavaScript_form = function() {
  this.radios = document.querySelectorAll('input[type="radio"]');
}

// Public method of instance
JavaScript_form.prototype.radio_hide_element = function() {

  // inputs that will be hidden
  var els_hidden = document.getElementsByClassName('radio-condition'),
      els_hidden_length = els_hidden.length;

  // loop each radio input
  [].forEach.call(this.radios, function (el) {
      var i = 0;
      for (; i < els_hidden_length; i++) {
        els_hidden[i].style.display = 'none';
      }
      els_hidden[0].style.display = 'block';

      // click radio will toggle visibility of text input
      el.addEventListener('click', function() {
        var i = 0;
        for (; i < els_hidden_length; i++) {
          els_hidden[i].style.display = 'none'; 
          els_hidden[i].value = '';
        }
        document.getElementById('value-' + this.value).style.display = 'block';
      });
  });
}

// Make instance
var JS_radios = new JavaScript_form();
JS_radios.radio_hide_element();

Answer 1

I'd use data attributes on the radio buttons to point to the IDs of the text inputs. Then you don't have to worry about constructing the ID strings in code. For IE8, use getAttribute()

I'd also use external CSS to default all the text fields to hidden -- to avoid that first nested loop in your JavaScript.

I'd then use a CSS class to toggle the visibility of the text fields (if the class is present, the field is visible, if the class is not present, the field is hidden). This makes it easier to locate the currently visible input (document.getElementsByClass), instead of having to iterate through all the text fields every time a radio button is clicked.

If you use data attributes and CSS classes, the event handler becomes pretty generic. You could use "event delegation" and attach the handler once to the parent element (avoiding that forEach loop -- which is not supported natively in IE8).

FYI, the <label> tag is significantly less usable without it either encompassing both the radio button and label text, or having its for attributed defined.

Answer 2

Form usability

According to the HTML specification, invisible <input> fields should still be submitted with the form. Judging from the fact that your JavaScript empties their values when hiding, you wish to suppress invisible fields. Perhaps it would be better to disable the element instead? That way, when a field is reactivated by selecting its associated radio button, any previously entered value will have been preserved instead of cleared. It might also be better from a semantic point of view to not submit a field at all than to submit a field with an empty value.

If you nest the <input> inside the corresponding <label>…

<label><input type="radio" name="color" value="yellow" checked />Yellow</label>

then clicking on the color name will also activate the radio button. This gives the user a larger clickable target.

forEach()

Admittedly, it took me a while to figure out what was going on with

[].forEach.call(this.radios, function(…) { … });

even though it is correct.

However, Array.forEach() is supported by Internet Explorer only starting with IE 9, as documented by MSDN and confirmed by Mozilla. If you wish to target IE 8, you can't use it.

But why is there a for loop at all inside the forEach callback? One level of looping should suffice.