3
\$\begingroup\$

Having a hard time to achieve what I'm looking for. I was thinking of using arrays but don't know where to start. Code below is what I want to shorten to get rid of multiple if statement using jQuery.

$("input#communication").on("slide", function(slideEvt) {
    if(slideEvt.value == 1) {
        $('#slider-val1').css('color', '#D0011B');
        $('.slider-label').not('#slider-val1').css('color', '#ccc');
        $('.slider-handle').css({
            'border': '5px solid #fff',
            'box-shadow': '0 0 0 2px #D0011B'
         })
        $("#desc").text('Amir is a terrible communicator.');
    }

    if(slideEvt.value == 2) {
        $('#slider-val2').css('color', '#D0011B');
        $('.slider-label').not('#slider-val2').css('color', '#ccc');
        $("#desc").text('Amir struggles, needs a lot of help to improve communication skills.');
    }

    if(slideEvt.value == 3) {
        $('.slider-handle').css({
            'border': '5px solid #fff',
            'box-shadow': '0 0 0 2px #D0011B'
        })
        $('.slider-handle').css('background', '#D0011B');
        $('.slider-selection').css('background', '#D0011B');
        $('#slider-val3').css('color', '#D0011B');
        $('.slider-label').not('#slider-val3').css('color', '#ccc');
        $("#desc").text('Amir needs some help to improve communication skills.');
    }

    if(slideEvt.value == 4) {
        $('.slider-handle').css('background', '#F47621');
        $('.slider-selection').css('background', '#F47621');
        $('#slider-val4').css('color', '#F47621');
        $('.slider-handle').css({
            'border': '5px solid #fff',
            'box-shadow': '0 0 0 2px #F47621'
        })
        $('.slider-label').not('#slider-val4').css('color', '#ccc');
        $("#desc").text('Amir has below average communication skills.');
    }

    if(slideEvt.value == 5) {
        $('#slider-val5').css('color', '#F47621');
        $('.slider-label').not('#slider-val5').css('color', '#ccc');
        $("#desc").text('Amir has adequate communications skills.');
    }
});

The code above use to change the $('#desc') text and CSS every time the slider is slide. Now, I'm looking for a good way on how to shorten thisif else statements.

\$\endgroup\$
2
  • \$\begingroup\$ Can you change/update your CSS and HTML as well or only the JavaScript? Could you add the HTML or at least the example of one slide to the question? \$\endgroup\$ – insertusernamehere Jun 8 '17 at 7:06
  • \$\begingroup\$ A good solution probably involves rewriting more than just the JavaScript. Could you show us the corresponding HTML as well, ideally with a live demo? (When editing the question, press Ctrl-M.) \$\endgroup\$ – 200_success Jun 8 '17 at 21:28
2
\$\begingroup\$

You are duplicating a lot of code here. You said you wanted an array, so make an array containing all the values that change and then use the slideEvt.value to recall the values from the array at the correct index..

Something like this maybe.. no if() statements required (at least not for the slideEvt.value):

$("input#communication").on("slide", function(slideEvt) {
    var slideEvtVals = [
      {color: '#D0011B', css: {'border': '5px solid #fff', 'box-shadow': '0 0 0 2px #D0011B'}, text: 'Amir is a terrible communicator.'}
      {color: '#D0011B', text: 'Amir struggles, needs a lot of help to improve communication skills.'}
      {color: '#D0011B', css: {'border': '5px solid #fff', 'box-shadow': '0 0 0 2px #D0011B'}, text: 'Amir needs some help to improve communication skills.', handleBG: '#D0011B', selBg: '#D0011B'}
      {color: '#F47621', css: {'border': '5px solid #fff', 'box-shadow': '0 0 0 2px #F47621'}, text: 'Amir has below average communication skills.', handleBG: '#F47621', selBg: '#F47621'}
      {color: '#F47621', text: 'Amir has adequate communications skills.'}
    ];

    var index = slideEvt.value-1;
    $('#slider-val'+slideEvt.value).css('color', slideEvtVals[index].color);
    $('.slider-label').not('#slider-val'+slideEvt.value).css('color', '#ccc');
    if(slideEvtVals[index].css) $('.slider-handle').css(slideEvtVals[index].css);
    $("#desc").text(slideEvtVals[index].text);
    $("#desc").text(slideEvtVals[index].text);
    if(slideEvtVals[index].handleBG) $('.slider-handle').css('background', slideEvtVals[index].handleBG);
    if(slideEvtVals[index].selBg) $('.slider-selection').css('background', slideEvtVals[index].selBg);
});
\$\endgroup\$
1
\$\begingroup\$

A simple improvement is to use else if instead of multiple ifs.

Taking out common statements in the each of if statement.

  1. $('#some-selector').css('color', '#some-color');
  2. $('.slider-label').not('#some-selector').css('color', '#ccc');
  3. $('#desc').text('Some Message');

By making the some... part dynamic, we can make this code common for all conditions.

For the message, we can use either object or array(in case of numeric keys) and store the message on that key.

The colors color, background & box-shadow can also made dynamic using below condition.

var color = value <= 3 ? '#d0011b' : '#f47621';

Now, the statements using these colors are dynamic and can be moved outside of their respective if statements.

After making all the common things dynamic, there need to add special conditions for case 1, 3 & 4.


Below is compete code with description comments:

// Array starts at zero'th index
var description = [undefined,
    'Amir is a terrible communicator.',
    'Amir struggles, needs a lot of help to improve communication skills.',
    'Amir needs some help to improve communication skills.',
    'Amir has below average communication skills.',
    'Amir has adequate communications skills.'
];

$("input#communication").on("slide", function(slideEvt) {
    // Get the value and convert to number
    var value = Number(slideEvt.value);
    // Dynamic color based on value
    var color = value <= 3 ? '#d0011b' : '#f47621';

    // Dynamic selectors based on value
    $('#slider-val' + value).css('color', color);
    $('.slider-label').not('#slider-val' + value).css('color', '#ccc');

    // Special handling for some values
    if (value === 1 || value === 3 || value === 4) {
        $('.slider-handle').css({
            'border': '5px solid #fff',
            'box-shadow': '0 0 0 2px ' + color
        });


        // Only for 3 & 4
        if (value === 3 || value === 4) {
            // Multiple selectors
            $('.slider-handle, .slider-selection').css('background', color);
        }
    }

    // Get message at that index
    $('#desc').text(description[value]);
});

I'll suggest to move the styles in CSS classes and add/remove the classes using JavaScript.

If ES6 is supported in the target environment, you can take advantage of its features like template literals.

\$\endgroup\$
0
0
\$\begingroup\$

Look into using a switch statement to replace all the ifs, and extract the parts that are the same except for the selector (like the CSS), and use a variable to create a string to pass to the selector. Instead of $('#slider-val5') use document.getElementById('slider-val' + slideEvt.value).

\$\endgroup\$
1
  • 1
    \$\begingroup\$ switch will also be long. I was thinking about making an array but thanks though. \$\endgroup\$ – claudios Jun 8 '17 at 6:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.