HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Review on my multiple JQuery Slider on a page

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sliderjquerypagemultiplereview

Problem

I have successfully placed multiple jquery ui slider on a page and its transalation text too. you can check it live on:
http://outsourcingnepal.com/projects/jQuery%20slider/

I have my jquery code as :

var arrLabel1 = new Array('Not Good', 'OK', 'Average', 'Above Average', 'Excellent', 'Outstanding');
$(function() {
    $( ".slider_control" ).slider({
        value:0,
        min: 0,
        max: 5,
        step: 1,
        slide: function( event, ui ) {
            $( "#slider_" + $(this).attr('rel') ).val( ui.value );
            $( "#label_" + $(this).attr('rel') ).text( arrLabel1[ ui.value ] );
        }
    }).each(function(index){
        $( "#slider_" + (index + 1) ).val( $( this ).slider( "value" ) );
        $( "#label_" + (index + 1) ).text( arrLabel1[ $(this).slider( "value" ) ] );
    });
});


Now i would like to optimize it so that i can be independent of rel attribute.

MY HTML:

 
     
        Overall:  
         
         
     
     
        Cleanliness:  
         
         
     
     
        Facilities:  
         
         
     
     
        Location:  
         
         
     
     
        Quality of Service:  
         
         
     
     
        Room:  
         
         
     
     
        Value of Money:  
         
         
     

Solution

In my option your code has two problems:

-
You unnecessarily fill your HTML with empty elements which you could just create in your script. Changing that would remove the necessity for the rel attribute.

-
more importantly (even if there are people that may disagree), you have unnecessarily made your form dependent on JavaScript. It would be much better to have select elements in your HTML, which you can replace with sliders. That way even users without JavaScript can use your form unrestricted.

I've written an example how I would do it:

$(".ui_slider select").each(function() {
    var select = $(this); // Cache a refernce to the current select
    var sliderDiv = $(""); // create a div for the slider
    var displayLabel = $(""); // create a span to display current selection

    if (select[0].selectedIndex < 0) // Make sure that an item is selected
        select[0].selectedIndex = 0;

    select
        .hide() // hide the select
        .before( // Insert display label before the select
            displayLabel
               .text(select.find("option:selected").text()) // and set it's default text
        )
        .after( 
           sliderDiv // Insert the silder div after the select
               .data("select", select) // store a reference to the select
               .data("label", displayLabel) // store a reference to the display label
               .slider({
                    max: select.find("option").length - 1, // set to number of items in select
                    slide: function(event, ui) {
                        var select = $(this).data("select"); 
                        select[0].selectedIndex = ui.value; // update the select
                        $(this).data("label").text( // Update the display label
                            select.find("option:selected").text()
                        );
                    }
               })
        );
});


HTML (repeat as needed):


    Overall: 
    
        Not Good
        OK
        Average
        Above Average
        Excellent
        Outstanding
    


Working sample: http://jsfiddle.net/YfqCx/2/

Code Snippets

$(".ui_slider select").each(function() {
    var select = $(this); // Cache a refernce to the current select
    var sliderDiv = $("<div></div>"); // create a div for the slider
    var displayLabel = $("<span></span>"); // create a span to display current selection

    if (select[0].selectedIndex < 0) // Make sure that an item is selected
        select[0].selectedIndex = 0;

    select
        .hide() // hide the select
        .before( // Insert display label before the select
            displayLabel
               .text(select.find("option:selected").text()) // and set it's default text
        )
        .after( 
           sliderDiv // Insert the silder div after the select
               .data("select", select) // store a reference to the select
               .data("label", displayLabel) // store a reference to the display label
               .slider({
                    max: select.find("option").length - 1, // set to number of items in select
                    slide: function(event, ui) {
                        var select = $(this).data("select"); 
                        select[0].selectedIndex = ui.value; // update the select
                        $(this).data("label").text( // Update the display label
                            select.find("option:selected").text()
                        );
                    }
               })
        );
});
<div class="ui_slider">
    <label for="overall">Overall: </label>
    <select name="overall" id="overall">
        <option value="0">Not Good</option>
        <option value="1">OK</option>
        <option value="2">Average</option>
        <option value="3">Above Average</option>
        <option value="4">Excellent</option>
        <option value="5">Outstanding</option>
    </select>
</div>

Context

StackExchange Code Review Q#1376, answer score: 5

Revisions (0)

No revisions yet.