0

I'm trying to create a JavaScript object for displaying messages. It takes a name, text to show, and an array of button objects as parameters. The array can have an arbitrary length, so the message can have 1 or more buttons. Each button object has a name to display and a function to execute when it is clicked. When the actual html is created, the onclick event of the button should be a function to destroy the message, plus the function of the button object itself.
The code below works just fine, but I'm not sure if this is good practice, since I don't really understand closures yet.

function createMessage(name, body, buttons) {
    this.name = name;
    this.body = body; // text to show in message
    this.buttons = buttons; // an array of button objects (button name, function to call when clicked)
}

createMessage.prototype.display = function () { // display the message
    this.msgElement = document.createElement("div");
    this.msgElement.appendChild(document.createTextNode(this.body));

    var btndiv = document.createElement("div"); // a container for the buttons
    btndiv.style.margin = "6px auto 6px auto";
    for (var i = 0; i < this.buttons.length; i++) {
        var btn = document.createElement("button");
        btn.onclick = function(msgObj,i){ // this looks like bad coding ...
            return function () {
                msgObj.remove();
                msgObj.buttons[i].onclickfn();
            }
        }(this,i);
        console.log(btn.onclick);
        btn.appendChild(document.createTextNode(this.buttons[i].name));
        btndiv.appendChild(btn);
    }
    this.msgElement.appendChild(btndiv);
    document.body.appendChild(this.msgElement); // actually insert the message into the webpage

    var msgObj = this;
    setTimeout(function(){ // start showing the message after a delay of 200ms and fade in
        msgObj.msgElement.style.opacity = "1";
    },200);
};

createMessage.prototype.remove = function () {
    // remove the message from the screen
    if(this.msgElement) console.log(this.name+" deleted.");
};

function createButton(name, onclickfn) {
    this.name = name;
    this.onclickfn = onclickfn;
}

The problem is combining the msgObj.remove() function with the onclickfn() function of the button object. (in the for-loop).
1. Is this bad code? If so, what are my alternatives?
2. The console.log(btn.onclick) always logs "function() {msgObj.remove();msgObj.buttons[i].onclickfn();}", even though this prints the same for every button, each button executes a different function. Does this mean that there are x different msgObj's and x different i vars?
3. To fade in the message, I use the setTimeout function. Since 'this' inside this function would refer to the function itself instead of the createMessage object, I use the variable msgObj (at the end of createMessage.prototype.display). Is this the right way to do this?

You can find the JSfiddle with a working code example here.

Thanks in advance,
Pieter

tttapa
  • 1,397
  • 12
  • 26
  • Please ask only one question per post. – Bergi Jul 07 '16 at 15:47
  • "*Is this bad code?*" - no, it's necessary code. See the duplicate. "*Does this mean that there are x different msgObj's and x different i vars?*" - Yes. "*I use the variable msgObj*" - yes, that's fine. – Bergi Jul 07 '16 at 15:48
  • Thanks for clearing it up! – tttapa Jul 07 '16 at 15:52

0 Answers0