Step-by-step form

Question

I am creating a step-by-step form (it only shows in step-by-step and is not a multi state form) and the parts slide in and out (similar to GitHub but without the browser's back buttons). I have written the following code to implement this. Could you please review my code and suggest flaws:

$(document).ready(function(){

  // when the div with text next is clicked
  $('.nextButton').click(function(){

    // remove the class from div that is visible at present, add it the class for divs on the left side and move it to the left side 
    // and remove from the next div, the class assigned divs lying to the right and add bring it in the visible area and assign visible class to it

    $('.visible').removeClass('visible').addClass('onLeft').animate({left:'-980px'}).next('div').removeClass('onRight').animate({left:'0px'}).addClass('visible');

    //fade in the div with title previous  
    $('.prevButton').fadeIn();

    //if there are no div left on the right side remove the class for next button and add the class for done button and modify the HTML to done
    if($('.onRight').length==0)
    {
      $(this).removeClass('nextButton').addClass('doneButton').html('Done');
    }
  });

  //when the div with text previous is clicked
  $('.prevButton').click(function(){

    //if the div with class 'doneButton' exists, i.e we are at the last step, then remove the class and assign the div class for next button and text next
    if($('.doneButton').length)
    {
      $('.doneButton').removeClass('doneButton').addClass('nextButton').html('Next');           
    }

    //if there is just 1 div left towards left, i.e. we are on the 2nd step, remove  the div with title previous
    if($('.onLeft').length==1)
    {
      $(this).fadeOut();
    }

    //remove the class from div that is visible at present, add to it the class for divs on right side and move it to the right side, remove 
    //the class assigned to divs on the left from the left div and add visible class to the div and bring it in the visible section
    $('.visible').removeClass('visible').addClass('onRight').animate({left:'980px'}).prev('div').removeClass('onLeft').animate({left:'0px'}).addClass('visible');
  });
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.7.2/jquery.min.js"></script>

<div class="formContent" style="width:980px;height:100px;float:left;overflow:hidden;position:relative;">
                <div class="visible bt" style="width:980px;float:left;position:absolute;">Part1</div>
                <div class="onRight bt" style="width:980px;float:left;position:absolute;left:980px;">Part2</div>
                <div class="onRight bt" style="width:980px;float:left;position:absolute;left:980px;">Part3</div>
                <div class="onRight bt" style="width:980px;float:left;position:absolute;left:980px;">Part4</div>
            </div>
            <div class="formButtons" style="width:980px;height:50px;float:left;">
                <div class="nextButton" style="float:right">Next</div>
                <div class="prevButton" style="float:right;margin-right:20px;display:none;">Previous</div>
            </div>

(Note that the "Next" button is quite far to the right in this demo.)

Answer 1

Did some mods with the HTML. You should separate classes for targetting and classes for state. You can use data-* to keep states also, just so you know. But for this one, I used leftButton and rightButton classes for targetting and left prevButton, nextButton and doneButton for the state.

<div class="formButtons" style="width:980px;height:50px;float:left;">
    <div class="nextButton rightButton" style="float:right">Next</div>
    <div class="prevButton leftButton" style="float:right;margin-right:20px;display:none;">Previous</div>
</div>​

As for the JS, it's pretty much commented. And jQuery was designed to be chained. However, you have to know what is returned in the previous call so that you can optimize the chain.

//shortened doc ready
$(function() {

    //cache the buttons. they never go away so set them statically
    //added an additional class for targetting, instead of the 
    //prev, next and done states
    var rightButton = $('.rightButton');
    var leftButton = $('.leftButton');

    //delegation using on()
    $('.formContent').on('click', '.rightButton', function() {

        //use the value as boolean
        if (!$('.onRight').length) {
            $(this)                        //you can get away with $(this) 
                .removeClass('nextButton') //because it's only used once
                .addClass('doneButton')
                .text('Done');             //use text if you are only putting text              
        }

        leftButton.fadeIn();

        $('.visible')                      //i usually newline and indent
            .removeClass('visible')        //anything that pertains to the current selection
            .addClass('onLeft')            //moved addClass up for readability only if it's for state.
            .animate({                     //if it's used for style
                left: '-980px'             //move it back below animate
            })                             
            .next()                        //you can omit the "div" since they are all divs
                .removeClass('onRight')    //indentation again
                .addClass('visible')
                .animate({
                    left: '0px'
                });


    }) //chain the next handler
    .on('click', '.leftButton', function() {

        //cache donebutton since it's used twice
        var doneButton = $('.doneButton');

        if (doneButton.length) {
            doneButton.removeClass('doneButton')
                .addClass('nextButton')
                .text('Next'); //use text if you are only putting text
        }

        //use the value as boolean
        if ($('.onLeft').length) {
            $(this).fadeOut();
        }

        $('.visible')
            .removeClass('visible')
            .addClass('onRight')
            .animate({
                left: '980px'
            })
            .prev() //same as before, omit the div
                .removeClass('onLeft')
                .addClass('visible')
                .animate({
                    left: '0px'
                });
    });
});​

Answer 2

jQuery was designed so you can chain function calls together like that. If you're worried about readability, then break them up into logical parts. You can either use variables, or keep the chaining and use whitespace to split them up. Instead of:

$('.visible').removeClass('visible').addClass('onLeft').animate({left:'-980px'}).next('div').removeClass('onRight').animate({left:'0px'}).addClass('visible');

you could do something like this:

$('.visible')
  .removeClass('visible')
  .addClass('onLeft')
  .animate({left:'-980px'})
.next('div')
  .removeClass('onRight')
  .animate({left:'0px'})
  .addClass('visible');