Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Here is my script of a simple jQuery-powered galler I've decided to create while studying JavaScript. Please point out my mistakes and let me know how the script can be optimized!

JavaScript

// define globals
var overL = $('div.overlay_p');
var temP = $('div.pre_load');
var imgM = $('span.p_img');
var fCont = $('p.aligner');
var allin = $('span.hold_all');
var imGal = $('.img_gallery');
var orImg = $('img.thumb');
var nextB = $('span.next_div');
var prevB = $('span.prev_div');


// function on img click
orImg.on('click', function () {
    clearAll();
    overL.fadeIn();
    currentImage = $(this);
    getDim($(this).attr('alt'));
});


// fucntion on NEXT or PREV click
$('span#nav').on('click', function () {
    if($(this).attr('class')=='next_div'){
    nextImg = currentImage.next('img');
    }else{
    nextImg = currentImage.prev('img');
    }
    var nextImgA = nextImg.attr('alt');
    currentImage = nextImg;
    imgM.fadeOut().delay(600).empty().fadeIn();
    getDim(nextImgA);
});


// get WIDTH and HEIGHT of the loaded IMAGE, and load into p_img
function getDim(element) {
    temP.html('<img src="' + element + '"/>');
    var dUrl = $('img', temP);
    checkButtons(currentImage);
    var imgW = dUrl.width();
    var imgH = dUrl.height();
    fCont.animate({
        width: imgW,
        height: imgH
    }, 600, function () {
        imgM.html('<img src="' + element + '"/>').parent('span').fadeIn(400);
    });
}


//clear cocntainers
function clearAll() {
    temP.empty();
    imgM.empty();
}

//HIDE relevnt button if NEXT or PREV img does not exist
function checkButtons(element) {
    if (element.prev('img').attr('alt') === undefined) {
        prevB.hide();
        nextB.show();
    } else if (element.next('img').attr('alt') === undefined) {
        nextB.hide();
        prevB.show();
    } else {
        nextB.show();
        prevB.show();
    }
}

//on CLOSE click
$('span.close_div').on('click', function () {
    allin.fadeOut(300, function () {
        fCont.fadeOut(function () {
            overL.fadeOut(function () {
                fCont.attr('style', '');
            });
            clearAll();
        });
    });
});

CSS

html, body {
    height:1000px;
    width:100%;
}
* {
    padding:0;
    margin:0;
}
a {
    text-decoration:none;
}
.overlay_p {
    position:fixed;
    width:100%;
    z-index:10;
    display:none;
    min-height:100%;
    background:url('http://s20.postimg.org/5c4ymoxah/image.png');
}
p.aligner {
    border-radius:4px;
    overflow:hidden;
    box-shadow:0 3px 6px #000;
    z-index:50;
    height:60px;
    width:60px;
    margin:150px auto;
    line-height:0;
    border:1px solid silver;
    position:relative;
    padding:8px;
    background:url('http://s20.postimg.org/r630mhhft/249_1.gif') center center no-repeat #fff;
    display:block;
}
p.aligner img {
    vertical-align:top;
}
span.close_div {
    font-family:Calibri;
    line-height:25px;
    padding:0 10px;
    right:0;
    cursor:pointer;
    top:0;
    background:#fff;
    color:#333;
    position:absolute;
}
span.next_div, span.prev_div {
    font-family:Calibri;
    line-height:25px;
    padding:0 10px;
    right:0;
    display:block;
    text-align:center;
    width:60px;
    cursor:pointer;
    margin-top:-13px;
    top:50%;
    background:#fff;
    color:#333;
    position:absolute;
}
span.prev_div {
    left:0;
}
.pre_load {
    position:fixed;
    top:-2000px;
}
.thumb {
    height:100px;
    width:150px;
}
.hold_all {
    display:none;
}
img_gallery a {
    display:table;
    float:left;
    position:relative;
}
img_gallery a img {
    float:left;
}
.click {
    border:2px solid red;
}

HTML

<div class="pre_load"></div>
<div class="overlay_p">
    <p class="aligner"> <span class="hold_all" id="all_here"> 
        <span class="p_img"></span>
 <span class="prev_div" id="nav">prev</span>  
 <span class="next_div" id="nav">next</span> 
 <span class="close_div">X</span>
</span>
    </p>
</div>
<div class="img_gallery">
    <img src="http://papermashup.com/wp-content/uploads/2009/07/gallery.png" class="thumb" alt="http://www.st-hughs.ox.ac.uk/__data/assets/image/0014/5153/Gym---Main-Room.jpg">
    <img src="http://2.bp.blogspot.com/-V3ImhEBNtS4/UJRC08KGp6I/AAAAAAAAAJc/8rMMG6VIbvQ/s320/Candy.jpg" class="thumb" alt="http://2.bp.blogspot.com/-V3ImhEBNtS4/UJRC08KGp6I/AAAAAAAAAJc/8rMMG6VIbvQ/s320/Candy.jpg">
    <img src="http://images.nationalgeographic.com/wpf/media-live/photos/000/201/cache/common-ancestor-all-creatures_20194_600x450.jpg" class="thumb" alt="http://images.nationalgeographic.com/wpf/media-live/photos/000/201/cache/common-ancestor-all-creatures_20194_600x450.jpg">
</div>
share|improve this question

1 Answer 1

$(this).hasClass('next_div')

is usually preferred over

$(this).attr('class')=='next_div'

jQuery object variables are usually prefixed with a dollar sign ($):

var $allin = $('span.hold_all');
share|improve this answer
    
Thx for a comment! Will try to follow your advice! –  Cone Enoc Jul 18 '13 at 8:13

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.