2
\$\begingroup\$
<div id="members">
    <div id="inputRow0">
        <input id="input0_0" class="input" type="text" /><br/>
        <input id="input0_1" class="input" type="text" /><br/>
        <input id="input0_2" class="input" type="text" />
    </div>
    <div id="inputRow1">
        <input id="input1_0" class="input" type="text" /><br/>
        <input id="input1_1" class="input" type="text" /><br/>
        <input id="input1_2" class="input" type="text" />
    </div>
    <div id="inputRow2">
        <input id="input2_0" class="input" type="text" /><br/>
        <input id="input2_1" class="input" type="text" /><br/>
        <input id="input2_2" class="input" type="text" />
    </div>
</div>  
<br /><a id="add-more" href="#">add-more</a> <a id="remove-last" href="#">remove-last</>

js part that we need to deal with:

$("#inputRow1").hide();
$("#inputRow2").hide();
$("#remove-last").hide();

$("#add-more").click(function(){

    //count how many *direct children* elements are hidden.
    var hiddenElements = $('#members >:hidden').length;

    $("#remove-last").show();    

    if (hiddenElements === 2) {
        $("#inputRow1").show();
    } else if(hiddenElements === 1) {
        $("#inputRow2").show();
        $(this).hide();
    }
});

$("#remove-last").click(function(){

    //count how many *direct children* elements are hidden.
    var hiddenElements = $('#members >:hidden').length;
    $("#add-more").show();    

    if (hiddenElements === 0) {
        $("#inputRow"+2).hide();
    } else if (hiddenElements === 1) {
         $("#inputRow"+hiddenElements).hide();
        $(this).hide();
    }

});

Working example: http://jsfiddle.net/URkuW/

\$\endgroup\$
3
\$\begingroup\$

What problem are you trying to solve? I've left your code doing the same things but rather than interrogate the DOM each time I've cached everything in local variables.

// Execute code in an Immediately Invoked Function Expression (IIFE)
// This helps stop your variables polluting the global scope.
(function () { 

    // opt in to ECMAScript 5's strict mode 
    "use strict";

    // As we aren't changing the DOM just store all of the selectors in 
    // local variables - it's cheaper than hitting the DOM each time.
    var inputRow1 = $('#inputRow1').hide(), 
        inputRow2 = $('#inputRow2').hide(),
        addRowLink = $('#add-more'),
        removeRowLink = $("#remove-last").hide(),
        // rather than checking the DOM for number of hidden children,
        // just keep track of the last row visible (0, 1 or 2)
        lastRowVisible = 0; 


    addRowLink.click(function(event) {
        // stop the default action of the click event.
        event.preventDefault();

        // This is the initial state with only the first 3 input boxes showing
        // we need to show row 1 and the remove last link as well as increment the
        // last row visible to 1.
        if (lastRowVisible === 0) {
            removeRowLink.show();
            inputRow1.show();
            ++lastRowVisible;
        // The first two rows are visible. Show the last row and hide the add-more
        // link as well as increment the last row visible to 2.
        } else if (lastRowVisible === 1) {
            addRowLink.hide();
            inputRow2.show();
            ++lastRowVisible;
        }   
    });

    removeRowLink.click(function(event){
        // stop the default action of the click event.
        event.preventDefault();

        // All 3 rows currently visible: need to hide the last row, show
        // the add-more link and decrement the last row visible to 1.
        if (lastRowVisible === 2) {
            addRowLink.show();
            inputRow2.hide();
            --lastRowVisible;

        // 2 rows currently visible: need to hide the second row and the
        // remove-last link and decrement the last row visible to 0.
        } else if (lastRowVisible === 1) {
            removeRowLink.hide();
            inputRow1.hide();
            --lastRowVisible;
        }   
    });   
}());

Here's a working fiddle: http://jsfiddle.net/RobH/CDVBs/1/

Have you considered a solution that starts with only 3 inputs and then creates extra inputs instead of showing ones that are already declared but hidden?

\$\endgroup\$
2
\$\begingroup\$

Don't feel intimidated by the following wall of text, I just like to over explain stuff sometimes. I think it's better to have more information than have too little.

First of all and probably the most important thing you could do to improve this code it to cache your selections. When you write this: $("#someElement"), jQuery has to jump into the DOM and look through all elements that would match that selection. Keep in mind that DOM manipulations are the most expensive when it comes to performance/resources. So you should really do a search only once, and save your results for future use.

Just put 'em in variables: var someElement = $(".someElement"); Then in your code just replace $(".someElement"); with someElement and do all your stuff on it. As a rule of thumb, if you use a selection more than once, you should cache it.

Best place, in my opinion, to do that is at the top of your code. By placing all your variables and selections at the top, you can see at a glance all the elements (or at least most of the elements) that are going to be used/manipulated in the code.

Next thing you could do is save some function calls. The .click() method you use if you read through the jQuery source code, you'll see that it simply calls the .on() method and passes in the stuff you set. It would be better if you just went straight to the point by using the .on("click", function(){}); directly. This way you save a few function calls. Not a tremendous improvement, but something that will benefit you in the long run.

In your click functions you also might want to prevent the default browser action on a link, which is to direct the page to that link. Since you just want to perform something on your page and don't actually want the browser to leave the page you should prevent that action. You can do it by passing in e for event and running e.preventDefault();. If you want to read more on this I would suggest this article by Chris Coyier.

Also since you are showing and hiding quite a bit, it might be hard to keep track of what is showed or hidden at any given time. I say don't worry about it. Let jQuery magically handle it by using the .toggle() method instead. If the element is hidden it will show it, but if it's not hidden then it will hide it. Simple and easy. If there's a point where you want to be specific about it you can always just use the methods you're using now.

Here is your code with these examples in them. Keep in mind that these are simple improvements which don't really change the order/logic of your code but just help you make it better. Anyways here:

var addMore = $("#add-more"), //Separate variables with comma
    removeLast = $("#remove-last"),
    //For these next three you could use members.children() 
    //or some other method and not have to do this at all
    inputRow0 = $("#inputRow0"),
    inputRow1 = $("#inputRow1"),
    inputRow2 = $("#inputRow2");

addMore.on("click", function(e) {
    //Prevent the default action
    e.preventDefault();

    //count hidden elements
    var hiddenElements = $('#members >:hidden').length;
    removeLast.toggle();

    if (hiddenElements === 2) {
        inputRow1.toggle();
    } else if (hiddenElements === 1) {
        inputRow2.toggle();
        $(this).hide();
    }
});

removeLast.on("click", function(e) {
    e.preventDefault();

    var hiddenElements = $('#members >:hidden').length;
    addMore.toggle();

    if (hiddenElements === 0) {
        inputRow2.toggle();
    } else if (hiddenElements === 1) {
         inputRow1.toggle();
        $(this).hide();
    }
});

If you wanted to change your code a bit here's an example with using child selectors. If you have question about any of these methods just let me know and I can explain them. You can also read up the jQuery documentation for each one to get a better idea of what's going on. Also here's a Fiddle: http://jsfiddle.net/jonnysooter/WuxgK/.

var addMore = $("#add-more"), //Separate variables with comma
    removeLast = $("#remove-last");

$("#members div:first-child").nextAll().hide();

addMore.on("click", function(e){
    e.preventDefault();
    //count hidden elems
    var hiddenElements = $('#members >:hidden').length +1;
    removeLast.show();

    $("#members div:nth-child(" + hiddenElements + ")").show();
    if(hiddenElements === 2){
        //Test for 2 because we add 1 in the variable
        $(this).hide();
    }
});

removeLast.on("click", function(e){
    e.preventDefault();

    var hiddenElements = $('#members >:hidden').length +2;
    addMore.show();

    $("#members div:nth-child(" + hiddenElements + ")").hide();
    if(hiddenElements === 3) {
        $(this).hide();
    }
});
\$\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.