2
\$\begingroup\$

When you click on a span it enters the value into the input. That works fine, but as you can see the JS code is very big and there must be a better way to do this, but like I said I'm still learning. Maybe someone can give me a pointer in the right direction.

I suppose I have to use a variable somewhere but can't really figure it out on my own.

jsFiddle

jQuery(document).ready(function() {

    $("#spanval1").click(function(){
        $('#hourvalue').val($('#spanval1').text());
    });

    $("#spanval2").click(function(){
        $('#hourvalue').val($('#spanval2').text());
    });

    $("#spanval3").click(function(){
        $('#hourvalue').val($('#spanval3').text());
    });

    jQuery('.hour_dropdown').hide()

    jQuery("#more").click(function() {
        $('.hour_dropdown').fadeToggle(200);
    });
});​
\$\endgroup\$
0
4
\$\begingroup\$

Building off of Danny's answer, if you have access to the HTML, you can add a class to each of your span tags and bind an event to that class:

<span id="spanval1" class="spanval">1</span>
<span id="spanval2" class="spanval">2</span>
<span id="spanval3" class="spanval">3</span>

The event call can become more generic by using the event's local context 'this'. From the .bind() documentation:

Within the handler, the keyword this refers to the DOM element to which the handler is bound. To make use of the element in jQuery, it can be passed to the normal $() function.

'this' inside of the click event will be the DOM element that was clicked on that caused the event to fire. Then you would need only the following to setup the click event for all of the elements with the class 'spanval':

$(".spanval").click(function(){
    $('#hourvalue').val($(this).text());
});

And with the rest of the script:

$(document).ready(function() {

   $(".spanval").click(function(){
       $('#hourvalue').val($(this).text());
   });

   $('.hour_dropdown').hide();

   $("#more").click(function() {
       $('.hour_dropdown').fadeToggle(200);
   });

});​
\$\endgroup\$
2
  • \$\begingroup\$ Typo alert: The first selector there should end with the letter L. \$\endgroup\$ – Joseph Silber Aug 6 '12 at 21:19
  • \$\begingroup\$ Thanks. But can you perhaps explain me how (this) knows the value of the span? \$\endgroup\$ – alucardu Aug 6 '12 at 22:20
2
\$\begingroup\$

First off, this refers to the element you clicked on, so we can simplify your code a bit my replacing your second #spanval like so:

$("#spanval1").click(function(){
    $('#hourvalue').val($(this).text());
});

Next, you can extract that click handler into it's own function, allowing the index to be passed in as a parameter.

function handleClick(num){
    $("#spanval"+num).click(function(){
        $('#hourvalue').val($(this).text());
    });
}

This will allow us to replacing all of those click handlers in the main function with this:

handleClick(1);
handleClick(2);
handleClick(3);

Now, we're still being repetitive, so lets reduce the repetition using a for loop:

jQuery(document).ready(function() {

    var maximumNumber = 3;
    for(var n = 1; n <= maximumNumber; n++){
        handleClick(n);
    }

    jQuery('.hour_dropdown').hide()

    jQuery("#more").click(function() {
        $('.hour_dropdown').fadeToggle(200);
    });
});​
\$\endgroup\$
0
\$\begingroup\$

Here are some points worth considering:

  1. Cache your selectors! If there's one thing you can do to increase performance, it's caching selectors. Traversing the DOM is an expensive operation, and should be done only when absolutely necessary.
  2. Use an attribute selector (span[id^=spanval]). This'll allow you to select all spans who's ID starts with spanval (if not all of them are spans, you can use the attribute selector on its own ([id^=spanval]) which is a tad slower but works just as well).

  3. Whenever possible, try not to use the jQuery constructor. It's much faster to call the static methods from the jQuery namespace (when they're available). So instead of $(this).text(), try using $.text(this).


With that in mind, here's some sample code:

jQuery(document).ready(function($) {

    var $hourVal = $('#hourvalue'),
        $hourDropdown = $('.hour_dropdown');

    $('span[id^=spanval]').click(function(){
        $hourVal.val( $.text(this) );
    });

    $hourDropdown.hide()

    $("#more").click(function() {
        $hourDropdown.fadeToggle(200);
    });
});​
\$\endgroup\$

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.