jQuery - Is there an easier way to write this?

Question

I am a new programmer so fogive me if this amateur... I am looking for some direction, or perhaps some ideas. I goal here is for me to learn, so any push in the right direction would be appreaciated.

ok.. I challenged myself to create a 'wizard' like control for a simple sign up form using jQuery. I can get through the steps quite nicely however I am looking at my code and I cant help but think; "there has to be a better, easier and proper way to do this". Here is what I have.

function toggleStep(){
        $("#nextButton").click(function(){
            if($("#nextButton").name = "step1"){
                $("#nextButton").attr("name", "step2");

                $("#backButton").attr("name", "step1").css("display", "inline");

                $("#step1").hide();
                $("#step2").show("fade", 250);
            }
            $("#nextButton").click(function(){
                if($("#nextButton").name = "step2"){
                    $("#nextButton").attr("name", "step3");

                    $("#backButton").attr("name", "step2");

                    $("#step2").hide();
                    $("#step3").show("fade", 250);
                }
                    $("#nextButton").click(function(){
                        if($("#nextButton").name = "step3"){
                            $("#nextButton").attr("name", "step4");
                            $("#nextButton").css("display", "none");

                            $("#backButton").attr("name", "step3");

                            $("#step3").hide();
                            $("#step4").show("fade", 250);
                        }
                });
            });
        }); 
    }

Also, I seem to have messed myself up when creating a "back button" function. This code is simply not good enough. How would you approach this? Thanks!!!

This is a better fit for http://codereview.stackexchange.com/. — Felix Kling, Dec 31 '12 at 00:19
Every single statement in your function contains a DOM query. (That's bad, btw `:P`) — Šime Vidas, Dec 31 '12 at 00:20
+1 for Felix, but I have two tips: you attach events too many times (attach events within event handler attached within some other event handler) and you select elements too many times (you could cache/save them into some variables and then reuse, unless you will need to select them again, because something has changed). — Tadeck, Dec 31 '12 at 00:22
http://stackoverflow.com/questions/5724400/does-using-this-instead-of-this-provide-a-performance-enhancement — Jared Farrish, Dec 31 '12 at 00:23
you could use `$(this)` keyword instead of `$("#nextButton")` in event handlers. — Okan Kocyigit, Dec 31 '12 at 00:25
It is ok, but think what will happen if you have 10 steps :D. Good luck, it's ok that you get yourself challenges. — stefanz, Dec 31 '12 at 00:38

score 4 · Answer 1 · answered Dec 31 '12 at 00:20

4

i would just use jquery to toggle an class="activeStep" and do the rest (hiding, showing, fading) with css.

answered Dec 31 '12 at 00:20

Bastian Rang

2,137
1
19
25

Thanks Boaz and stefanz, looking at your examples helped a lot... I am thrown off by this part though: var t = $(this), – Frank Castle Dec 31 '12 at 02:43

Boaz · Answer 2 · 2012-12-31T01:38:41.087

You'd probably want to look into jQuery's prev() and next() methods. Combined with a logic based on a class name for the steps (instead of DOM ids), these methods could very easily shorten and simplify your code.

A rough verbose example:

$('#nextButton').click(
    function() {
        var current_step = $('#steps_container').find('.step:visible');
        var next_step    = current_step.next();
        if (next_step.length) {
            current_step.hide();
            next_step.fadeIn(250);
        }
    }
);

$('#prevButton').click(
    function() {
        var current_step = $('#steps_container').find('.step:visible');
        var prev_step    = current_step.prev();
        if (prev_step.length) {
            current_step.hide();
            prev_step.fadeIn(250);
        }

    }
);

This should handle nicely any number of steps in the following markup:

<div id="steps_container">
    <div class="step">Step 1</div>
    <div class="step">Step 2</div>
    <div class="step">Step 3</div>
    <div class="step">Step 4</div>
    <div class="step">Step 5</div>
    <div class="step">Step 6</div>
    ...
</div>

As long as @stefanz is improving on my code, an even simpler and more generalized approach can be to bind the navigation buttons' handlers to a class as well, like so:

$('.steps_nav_buttons_bar .steps_nav_button').click(
    function() {
        var current_step = $('#steps_container').find('.step:visible'),
            new_step     = $(this).hasClass('next') ? current_step.next() : current_step.prev();

        if (new_step.length) {
            current_step.fadeOut('fast',function() { new_step.fadeIn('fast') })
        }
     }
);

The advantage of this is that it allows you to add more than one set of navigation buttons, for instance, in case you want navigation bars at the top and bottom:

<div class="steps_nav_buttons_bar top_bar">
    <div class="steps_nav_button prev">Prev</div>
    <div class="steps_nav_button next">Next</div>
</div>

<div id="steps_container">
    <div class="step">Step 1</div>
    <div class="step">Step 2</div>
    <div class="step">Step 3</div>
    <div class="step">Step 4</div>
    <div class="step">Step 5</div>
    <div class="step">Step 6</div>
    ...
</div>

<div class="steps_nav_buttons_bar bottom_bar">
    <div class="steps_nav_button prev">Prev</div>
    <div class="steps_nav_button next">Next</div>
</div>

score 2 · Accepted Answer · answered Dec 31 '12 at 01:05

I edited @Boaz code and now you get something clean, which i guess can help you also to understand better. I also added some comments.

The jQuery code become

 $('#next, #prev').click(function(){
    var t = $(this),
        current = $('#steps_container').find( '.step:visible' ),
        stepGo = ( t.attr( 'id' ) == 'next' ? current.next() : current.prev() );

    if ( stepGo.length ) {
      current.fadeOut( 'slow', function(){

        stepGo.fadeIn( 'slow' );
      });
    };
    return false;
  });

jQuery - Is there an easier way to write this?

3 Answers3