Using :last-child and :first-child with jQuery

Question

I'm trying to write my first self-built jQuery script, as a super simple gallery.

CSS:

#gallery li{
        display:none;
    }

    #gallery li.current {
        display: block;
    }

HTML:

<div id="gallery">
    <li class="current"><img src="img/1.jpg" /></li>
    <li><img src="img/2.jpg" /></li>
    <li><img src="img/3.jpg" /></li>
    <li><img src="img/4.jpg" /></li>
</div>
<div id="controls">
    <span id="left">&#60;</span>
    <span id="right">&#62;</span>
</div>

JQUERY:

$(function(){
    $('#controls #left').click(function(){
        var old = $('li.current');
        $('li.current').removeClass('current');
        if (old = $('#gallery:first-child')){
            $('#gallery:last-child').addClass('current');
        }else{
        $(old).prev('li').addClass('current');
        }
    });
    $('#controls #right').click(function(){
        var old = $('li.current');
        if (old = $('#gallery:last-child')){
            $('#gallery:first-child').addClass('current');
        }else{
        $('li.current').removeClass('current');
        $(old).next('li').addClass('current');
        }
    });
});

It worked fine until I added the if/else statements to check whether we're looking at the first/last gallery image. I want this statement so that the gallery loops. Can't figure out why it's not working, and debug is giving me nothing.

Can I get a little push in the right direction?

EDIT: using some suggestions below, here's where I'm at:

$(function(){
    $('#left').click(function(){
        var old = $('li.current');
        old.removeClass('current');
        if (old == $('#gallery li:first-child')){
            $('#gallery li:last-child').addClass('current');
        }else{
        old.prev('li').addClass('current');
        }
    });
    $('#right').click(function(){
        var old = $('li.current');
        old.removeClass('current');
        if (old == $('#gallery li:last-child')){
            $('#gallery li:first-child').addClass('current');
        }else{      
        old.next('li').addClass('current');
        }
    });
});

Still isn't looping around though (pressing "left" on the first image makes the gallery blank, as does pressing "right" on the last image)

Answer 1

jsBin demo

var c = 0; 
var imgN = $('#gallery li').length; // 4   
$('#left, #right').click(function(){
  c = this.id=='right' ? ++c : --c ;
  $('#gallery li').hide().eq( c%imgN ).show(); 
}); 

Or even:

var c = 0; 
$('#left, #right').click(function(){
  $('#gallery li').hide().eq( (this.id=='right'?++c:--c) % 4 ).show(); 
}); 

gallery Demo

P.S: instead of .hide() you can use .removeClass('current')
and instead of .show() you can use .addClass('current')

Answer 2

You can use .is(':first-child) and .is(':last-child') to find out if something is the first or last child respectively:

$('#controls #left').click(function() {
    var old = $('li.current')
        .removeClass('current');

    if (old.is(':first-child')) {
        // search the siblings for the last child
        old.siblings(':last-child').addClass('current');
    } else {
        $(old).prev('li').addClass('current');
    }
});

$('#controls #right').click(function() {
    var old = $('li.current')
        .removeClass('current');

    if (old.is(':last-child')) {
        old.siblings(':first-child').addClass('current');
    } else {
        $(old).next('li').addClass('current');
    }
});

​ Btw, I would change your HTML to this:

<div id="controls">
    <span class="left">&#60;</span>
    <span class="right">&#62;</span>
</div>

And then use $('#controls .left') and $('#controls .right') to target your clickable elements.

Edit

A fancier approach could be this, food for thought:

// wrap the functionality inside an anonymous function
// localizing the gallery and controls blocks
(function($gallery, $controls) {
    var pos = 0;

    // this function performs the actual move left and right, based on event data being passed in via evt.data.delta
    function move(evt) {
        var $items = $gallery.children(), // if the gallery never changes size you should cache this value
        max = $items.length;

        $items.eq(pos).removeClass('current');
        pos = (pos + evt.data.delta) % max; // update the position
        $items.eq(pos).addClass('current');
    }

    // attach the controls
    $controls
        .on('click', '.left', {
            delta: -1 // left decreases the position
        }, move)
        .on('click', '.right', {
            delta: 1 // right increases the position
        }, move);

    // make sure the first is always selected
    $gallery.children().removeClass('current').eq(pos).addClass('current');

}($('#gallery'), $('#controls')));​​​​​​​​​ // immediate invocation

Answer 3

You might want to be using ==:

if (old == $('#gallery:first-child')){

and

if (old == $('#gallery:last-child')){

Although that won't work anyways, as pointed out by Jack, because they are separate jQuery calls that may select the same elements and return similar jQuery objects, but won't work with comparison because they are separate collections of DOM element(s). The correct way is to use .is like I provide below...or to compare with:

if (old[0] == $('#gallery:first-child')[0]){

Indexing them like that retrieves the first item each in the selected sets, which are the actual DOM elements, and can be compared. Probably not preferred, but its not wrong.

Here's an example of proving this is true: http://jsfiddle.net/4adSj/

You're also doing a few weird things. You don't need to use this selector: $('#controls #right') - $('#right') is unique enough thanks to how id should be used.

Also, you store var old = $('li.current'); and then the next line you don't bother using old - you re-get it like $('li.current').

Also, you are nesting <li> elements in a <div>, when they should only be nested in <ul>, <ol>, or <menu>.

One more - the variable old is a jQuery object, so you don't need to be doing $(old) every other time you want to access it after you declared it. Just use old.jQueryMethod().

Finally, the other problem is that your selector for the if statements:

$('#gallery:last-child')

Means find the element that has the id "gallery" and is a last-child. You probably want:

$('#gallery > li:last-child')

Which means find the child element that is the last li child element of the parent element with an id of "gallery".

So here's my suggestion of your final code to use:

$(function(){
    $('#left').on("click", function () {
        var old = $('li.current');
        old.removeClass('current');
        if (old.is(':first-child')){
            $('#gallery > li:last-child').addClass('current');
        } else {
            old.prev('li').addClass('current');
        }
    });

    $('#right').on("click", function () {
        var old = $('li.current');
        old.removeClass('current');
        if (old.is(':last-child')){
            $('#gallery > li:first-child').addClass('current');
        } else {
            old.next('li').addClass('current');
        }
    });
});