0

In a for-loop commented as:

// Iterate over the array and append each element as li

... there is a closure which works fine if I just put console.log(tags[x]); which would suggest that the closure works as intended. However, when I put the actual code that the closure should contain, ie. this.displayImg(tags[x]); it throws an error. When I click on li, I get:

Uncaught TypeError: this.displayImg is not a function
(anonymous function) @ script.js:49

Please advise how I can access this.displayImg from that place.

var myGallery = (function () {
    var s;

    return {
        settings: {
            data: JSON.parse(data),
            filter: document.getElementById("filter"),
            gallery: document.getElementById("gallery")
        },
        init: function () {
            // kick things off
            s = this.settings;
            // By default, call the fillImages function with 'all' tag
            this.createFilters("all");
        },
        createFilters: function (tag) {
            // Load the image data
            // I made imgs global as I couldn't access it from displayImg
            imgs = this.settings.data.images;

            // Get image tags and generate a tag array
            var tags = ["all"];

            for (var i = 0; i < Object.keys(imgs).length; i++) {
                // Check if a tag is already in tags
                if (tags.indexOf(imgs[i].tag) > -1) {
                    // Yes, it is, so don't add it
                } else {
                    // No, it isn't, so add it to 'tags'
                    tags.push(imgs[i].tag);
                }
            }
            // Create an unordered list, assign a class to it and append to filter
            var ul = document.createElement('ul');
            ul.classList.add("ul-bare");
            this.settings.filter.appendChild(ul);

            // Iterate over the array and append each element as li
            for (var i = 0; i < tags.length; i++) {
                var li = document.createElement('li');
                ul.appendChild(li);
                li.innerHTML = tags[i];
                li.onclick = (function (x) {
                    return function () {
                        console.log(tags[x]);
                        this.displayImg(tags[x]);
                    };
                })(i);
            }
        },
        displayImg: function (filter) {
            // Add images to #gallery
            for (var i = 0; i < Object.keys(imgs).length; i++) {
                var div = document.createElement("img");
                // If the tage is 'all', display all images
                if (filter === "all") {
                    div.src = imgs[i].src;
                    this.settings.gallery.appendChild(div);
                } else {
                    // Display only images of certain category (tag argument)
                    if (imgs[i].tag === filter) {
                        div.src = imgs[i].src;
                        this.settings.gallery.appendChild(div);
                    }
                }
            }
        }
    };
})();

myGallery.init();

Thank you

John Weisz
  • 30,137
  • 13
  • 89
  • 132
Wasteland
  • 4,889
  • 14
  • 45
  • 91

2 Answers2

6

Inside the onclick, this will be a reference to the DOM element that was clicked, not what this means where you're hooking it up.

If you want to access the object this refers to when you're setting up the handler, use a variable:

var thisObj = this;
for (...) {

...and then in the handler:

thisObj.displayImg(tags[x]);

Although in this case, since you don't need this to refer to the handler, you can make it a lot simpler with Function#bind:

for (var i = 0; i < tags.length; i++) {
    var li = document.createElement('li');
    ul.appendChild(li);
    li.innerHTML = tags[i];
    li.onclick = function(x) {
        console.log(tags[x]);
        this.displayImg(tags[x]);
    }.bind(this, i);
}

Function#bind returns a new function that, when called, will call the original with a specific this value and any further arguments you give bind (followed by any arguments it receives at runtime).


Side note: You may as well just pass the tag in, rather than the index:

var thisObj = this;
for (var i = 0; i < tags.length; i++) {
    var li = document.createElement('li');
    ul.appendChild(li);
    li.innerHTML = tags[i];
    li.onclick = (function(tag) {
        return function() {
            console.log(tag);
            thisObj.displayImg(tag);
        };
    })(tags[i]);
}

or

for (var i = 0; i < tags.length; i++) {
    var li = document.createElement('li');
    ul.appendChild(li);
    li.innerHTML = tags[i];
    li.onclick = function(tag) {
        console.log(tag);
        this.displayImg(tag);
    }.bind(this, tags[i]);
}
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
2

Inside handler this reference the DOM element. I would use bind which is also more readable solution for passing value of i:

li.onclick = function(x) {            
        console.log(tags[x]);
        this.displayImg(tags[x]);            
    }.bind(this, i);