jQuery range slider change text based on hidden field value

By Posted on

Problem

I am working on a jQuery ratings function that uses the Foundation range slider. My current implementation works correctly on 2 of 7 of my range sliders. However, I am quickly realising that my implementation will become bloated as I have to repeat myself for each of the 7 range sliders.

HTML: 

<div class="row">
  <div class="small-8 medium-8 columns">
    <div class="range-slider" data-slider data-options="start: 1; end: 5;">
      <span class="range-slider-handle" role="slider" tabindex="0"></span>
      <span class="range-slider-active-segment"></span>
      <%= f.hidden_field :quality %>
    </div>
  </div>
  <div class="small-4 medium-4 columns">
    <span id="sliderOutput1">Average</span>
  </div>
</div>

<div class="row">
  <div class="small-8 medium-8 columns">
    <div class="range-slider" data-slider data-options="start: 1; end: 5;">
      <span class="range-slider-handle" role="slider" tabindex="0"></span>
      <span class="range-slider-active-segment"></span>
      <%= f.hidden_field :communication %>
    </div>
  </div>
  <div class="small-4 medium-4 columns">
    <span id="sliderOutput2">Average</span>
  </div>
</div>

jQuery:

$(document).on("page:load ready", function() {
  function ratings(selector, callback) {
    var input = $(selector);
    var oldvalue = input.val();
    setInterval(function(){
      if (input.val()!= oldvalue){
        oldvalue = input.val();
        callback();
      }
    }, 100);
  }
  ratings('input#agent_score_quality', function() {
    var inputValue = $('input#agent_score_quality').val();

    if(inputValue === '1') { // Terrible
      $("span#sliderOutput1").fadeOut(function() {
        $(this).text('Terrible').fadeIn();
      });
    }
    if(inputValue === '2') { // Poor
      $("span#sliderOutput1").fadeOut(function() {
        $(this).text('Poor').fadeIn();
      });
    }
    if(inputValue === '3') { // Average
      $("span#sliderOutput1").fadeOut(function() {
        $(this).text('Average').fadeIn();
      });
    }
    if(inputValue === '4') { // Good
      $("span#sliderOutput1").fadeOut(function() {
        $(this).text('Good').fadeIn();
      });
    }
    if(inputValue === '5') { // Excellent
      $("span#sliderOutput1").fadeOut(function() {
        $(this).text('Excellent').fadeIn();
      });
    }
  });
  ratings('input#agent_score_communication', function() {
    var inputValue = $('input#agent_score_communication').val();

    if(inputValue === '1') { // Terrible
      $("span#sliderOutput2").fadeOut(function() {
        $(this).text('Terrible').fadeIn();
      });
    }
    if(inputValue === '2') { // Poor
      $("span#sliderOutput2").fadeOut(function() {
        $(this).text('Poor').fadeIn();
      });
    }
    if(inputValue === '3') { // Average
      $("span#sliderOutput2").fadeOut(function() {
        $(this).text('Average').fadeIn();
      });
    }
    if(inputValue === '4') { // Good
      $("span#sliderOutput2").fadeOut(function() {
        $(this).text('Good').fadeIn();
      });
    }
    if(inputValue === '5') { // Excellent
      $("span#sliderOutput2").fadeOut(function() {
        $(this).text('Excellent').fadeIn();
      });
    }
  });
});

How can I improve this code so I don’t have to repeat the same thing over and over just with different selectors? My idea is to use a single selector for the sliderOutput and use the .closest() method to target each and insert my text, but I’m new to jQuery, so could I be way off?

Solution

First of all you can dramatically simplify your callback function body structure.
Instead of using a series of if (inputValue === ... you might define a simple array of your texts, like:

var texts = [
    'Terrible',
    'Poor',
    'Average',
    'Good',
    'Excellent'
];

Then the callback function entire body becomes as simple as:

// here the example of your first ratings() invocation
$('span#sliderOutput1').fadeOut(function() {
    $(this).text(texts[$('input#agent_score_quality').val() - 1]).fadeIn();
});

Now to avoid rewriting such a callback function for each of your rows, you can make it a named, independent function, then invoke it from your `setInterval() callback, with the needed information about involved row, so the whole JS part looks like this:

$(document).on("page:load ready", function() {

var texts = [
    'Terrible',
    'Poor',
    'Average',
    'Good',
    'Excellent'
];

  function ratings(selectorInput, selectorSlider) {
    var input = $(selectorInput);
    var slider = $(selectorSlider);
    var oldvalue = input.val();
    setInterval(function(){
      if (input.val()!= oldvalue){
        oldvalue = input.val();
        ratingsCallback(input, slider);
      }
    }, 100);
  }

  function ratingsCallback(input, slider) {
    $(slider).fadeOut(function() {
      $(this).text(texts[input.val() - 1]).fadeIn();
    });
  }

  ratings('input#agent_score_quality', 'span#sliderOutput1');
  ratings('input#agent_score_communication', 'span#sliderOutput2');
  // and so on...
});

Finally better solution

(but see also CAVEAT below)

Thanks to combined suggestions from @Chococroc comments and @Margus answer, this is a yet much more minified version of the code.

It only needs that each <span id="sliderOutput..."> element becomes <span class="slider-output">.
Then the whole JS part can be reduced to:

$(document).on("page:load ready", function() {

  var texts = [
      'Terrible',
      'Poor',
      'Average',
      'Good',
      'Excellent'
  ];

  $('.row input').on('change', function() {
    var slider = $(this).closest('.row').find('.slider-output');
    slider.fadeOut(function() {
      slider.text(texts[input.val() - 1]).fadeIn();
    });
  }
});

Note that this solution only focuses on the initial question main issue. Beyond that, as noticed by @Chococroc, it could be enhanced in several ways, like:

  • allow strict independency between logics and presentation, by using
    dedicated class names like js-slider-output, so CSS needs don’t mix
    with JS needs
  • ensure to avoid any potential conflict with other
    class="row" elements in the whole context, by affecting a
    supplemental class to each row, e.g. <div class="row js-slider">,
    so the change-event binding becomes $('.js-slider input')...

CAVEAT

Still focused on the main issue (how to reduce JS code) I didn’t pay attention to a particular point in the given HTML example: here the source data for the range values resides in hidden elements.

So as pointed out by the OP author, in this case the change event will not fire when value changes.

So what?

  • from the fact that these elements are hidden, we can assert that the changes don’t come from user action
  • then we can assume that some other JS part is involved to make this change to happen
  • so we should have the availability to add <this element>.change(); somewhere in this JS part

Hopefully not forgetting some other blocking point…

First of all do not use:

<span id="sliderOutput1">Average</span>

Instead use a class

<span class="sliderOutput">Average</span>

id’s are meant to be unique.

Second of all, you can subscribe to the change event, by asking data sliders change event. To get the value you can ask for input element value.

$('[data-slider]').on('change', function(){
    var sliderValue = $('.range-slider input').val();
    //...
});

Finally to set the value of element that shares a parent you can use jQuery parent().

Leave a Reply

Your email address will not be published. Required fields are marked *