CODEWITHSUNDEEP
jquery
general exception
general exception

Reputation: 4332

jQuery slideUp element A if visible then slideDown element B

I have the following HTML form:-

<form id="subform">
    <p>
        <input type="button" id="btn" value="Go!"/>
    </p>
    <p>
        <select id="optlist">
            <option value="1">Option 1</option>
            <option value="2">Option 2</option>
            <option value="3">Option 3</option>
        </select>
    </p>
</form>

And the following jQuery snippet:-

$("#btn").click(function () {
    // if address select list is visible slide it up and then slide down new <p> tag
    if ($('#optlist').is(":visible")) {
        $('form > p:nth-child(2)').slideUp('slow', function () {
            $('<p id="noresults">No results found</p>').insertAfter('form > p:nth-child(2)').hide().slideDown("slow");
        });
    }
    // else if select list is not visible just slide down the new <p> tag
    else {
        $('<p id="noresults">No results found</p>').insertAfter('form > p:nth-child(2)').hide().slideDown("slow");
    }
});

What I want to do is, if #optlist is currently visible slideUp() THEN slideDown() a new <p> tag, otherwise just slideDown() a new <p> tag (if the #optlist is currently hidden).

This works fine above, although it's a bit clunky, for example I have to repeat the insertion of the new <p> in both the if and the else statement.

Also in addition to this, there is a small bug, if the #noresults <p> tag already exists it will currently add another one, how could i get it to flash the current #noresults if it already exists ?

I could check to see if it exists using if ( $("#noresults").length ) {}; but i would also need to repeat this (in the if and else).

I could put the checking and creation of the new <p> in a function and make a call to it.

Is there a better way of structuring the current code ?

Fiddle here

Upvotes: 2

Views: 5311

Answers (2)

Mike Brant
Mike Brant

Reputation: 71384

It seems that since the desired behavior is always to hide the #optlist and show the #noresults div. That you could do something like this:

The HTML:

<form id="subform">
    <p>
        <input type="button" id="btn" value="Go!"/>
    </p>
    <p id="optlist_container">
        <select id="optlist">
            <option value="1">Option 1</option>
            <option value="2">Option 2</option>
            <option value="3">Option 3</option>
        </select>
    </p>
    <p id="noresults" style="display: none;">No results found</p>
</form>

And for jQuery:

$('#btn').click(function() {
    $('#optlist_container').slideUp('slow', function() {
        $('#noresults').slideDown('slow');
    });
});

I don't see any reason to check for the visibility of #optlist if you are always going to be hiding it anyways. And you can easily just have the #noresults element in place to begin with so you have no reason to continue to append it.

Upvotes: 4

Michael Geary
Michael Geary

Reputation: 28860

You never have to repeat code. You can always move repeated code to a function.

For example, making the smallest change to your code to avoid the repetition:

function noResults() {
    $('<p id="noresults">No results found</p>')
        .insertAfter('form > p:nth-child(2)')
        .hide()
        .slideDown("slow");
}

$("#btn").click(function() {
    // if address select list is visible slide it up and then slide down new <p> tag
    if ($('#optlist').is(":visible")) {
        $('form > p:nth-child(2)').slideUp('slow', noResults);
    }
    // else if select list is not visible just slide down the new <p> tag
    else {
        noResults();
    }
});

I also took the long jQuery chain in the repeated code and reformatted it for readability. Removing the repetition makes that more feasible.

Upvotes: 1

Related Questions