0

I'm building a (very) mini JS framework to use for customizing my eBay listings. I do know how to circumvent their "no remote scripts" policy, and I could use jQuery, but this is really an exercise in getting myself better at JS.

I have a function of the global window object which returns a bunch of methods, like this:

window.iq = (function(){
    return {
        tag: function(tag) {
            return document.getElementsByTagName(tag);
        },

        map: function(el, attr) {
            var arr = [];
            el = iq.tag(el);
            for (i = 0; i < el.length; i++) {
                var x = el[i].getAttribute(attr);
                    arr.push(x);
            }
            return arr;
        },

        // A bunch of others like this
    };
})();

I'm having trouble (by which I mean I'm utterly stuck) iterating through an array of data-name attributes and hiding or showing images based on whether there is a match. Here's the function:

iq.click('#mblThumbs img', function(){
    var dn = iq.map('img', 'data-name');
    for (i = 0; i < dn.length; i++) {
        if (this.getAttribute('data-name') === dn[i]) {
            iq.fadeOut(200, iq.sel('#mblSlide img:not([data-name="' + dn[i] + '"])'));
            iq.fadeIn(200, iq.sel('#mblSlide img[data-name="' + dn[i] + '"]'));
        }
    }
});

I can cycle through the first two images as many times as my heart desires, but as soon as I click on anything past the second image, the function only continues to work for it, and the subsequent indexes in the array. I'm guessing that this is either a problem with my map method, or maybe something to do with array length? I don't know. I'm flummoxed. Any thoughts or suggested are much appreciated.

FIDDLE: http://jsfiddle.net/h8z7c/

2
  • 2
    Your iteration variables (both named i) are global! Declare them as local with var. Fix that first.
    – Bergi
    Commented Jun 16, 2014 at 0:34
  • Of course you're right. That's not affecting the code I have currently, but it's sloppy work. Thanks. Again :)
    – Justin
    Commented Jun 16, 2014 at 0:57

1 Answer 1

2

The issue is indeed with your click callback. Your fadeout selector is finding the first image that is not data-name=dn[i], which is always either "one" (if you clicked 2), or "two" (if you clicked 1). You either need to use selAll to grab all of the elements that are not the clicked one, or keep track of which one is currently selected. Here are the two ways of fixing it.

// Make sure they are all hidden
iq.click('#mblThumbs img', function(){
var dn = iq.map('img', 'data-name');
for (i = 0; i < dn.length; i++) {
    if (this.getAttribute('data-name') === dn[i]) {
        var outs = iq.selAll('#mblSlide img:not([data-name="' + dn[i] + '"])');
        for (var j = 0; j < outs.length; j++) {
            iq.fadeOut(200, outs[j]);
        }
        iq.fadeIn(200, iq.sel('#mblSlide img[data-name="' + dn[i] + '"]'));
    }
}

});

// Or keep track of the currently selected
var selected = "one";
iq.click('#mblThumbs img', function(){
    var dn = iq.map('img', 'data-name');
    for (i = 0; i < dn.length; i++) {
        if (this.getAttribute('data-name') === dn[i] && dn[i] !== selected) {
            iq.fadeOut(200, iq.sel('#mblSlide img[data-name="' + selected + '"]'));
            iq.fadeIn(200, iq.sel('#mblSlide img[data-name="' + dn[i] + '"]'));
            selected = dn[i];
        }
    }
});
1
  • Wow, I think I'm just fried. Not only is that very clear now that I see it written out, I actually tried that and abandoned it, because it didn't work at the time. I must've had the syntax wrong. Thanks a ton! Going with the first example, by the way.
    – Justin
    Commented Jun 16, 2014 at 0:59

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