1

I have these javascript files. It is really small test project to understand how MVC works with javascript. When I try to click 'Delete Bloak' button, nothing happens except with last button, which has id of 100. The other buttons do not have any click event attached to them. Why?

Controller.js

    var BloakController = function BloakController(model, view) {
    this.model = model;
    this.view = view;
}

BloakController.prototype.init = function init() {
    this.view.onAddBloak = this.addBloak.bind(this);

    this.view.onDeleteBloak = this.deleteBloak.bind(this);

    this.view.onGetAllBloaks = this.getAllBloaks.bind(this);
}

BloakController.prototype.addBloak = function addBloak() {

}

BloakController.prototype.getAllBloaks = function getAllBloaks() {
    this.model.getAllBloaks(this.getAllBloaksList.bind(this));
}

BloakController.prototype.getAllBloaksList = function getAllBloaksList(allBloaksList) {
    for (var i = 0; i < allBloaksList.length; i++) {
        var bloak = allBloaksList[i];
        var bloakDataObjectModel = {
            title: bloak.title,
            body: bloak.body,
            userId: bloak.userId,
            id: bloak.id
        };

        this.view.render(bloakDataObjectModel);
    }
}

BloakController.prototype.deleteBloak = function deleteBloak(id) {
    this.model.deleteBloak(id, this.getAllBloaksList.bind(this));
}

Model.js

var BloakModel = function BloakModel(XMLHttpRequest) {
    this.XMLHttpRequest = XMLHttpRequest;
}

BloakController.prototype.addBloak = function addBloak() {

}

BloakModel.prototype.getAllBloaks = function getAllBloaks(callback) {
    var request = new this.XMLHttpRequest;

    request.onload = function onLoad(e) {
        var ajaxResponse = JSON.parse(e.currentTarget.responseText);
        callback(ajaxResponse);
    }

    request.open('GET', 'https://jsonplaceholder.typicode.com/posts', true);
    request.send();
}

BloakModel.prototype.deleteBloak = function deleteBloak(id, callback) {
    console.log(id);
    var request = new this.XMLHttpRequest;

    request.onload = function onLoad(e) {
        var ajaxResponse = JSON.parse(e.currentTarget.responseText);
        callback(ajaxResponse);
    }

    request.open('DELETE', 'https://jsonplaceholder.typicode.com/posts/' + id, true);
    request.send();
}

View.js

var BloakView = function BloakView(domElement) {
    this.domElement = domElement;

    this.onAddBloak = null;
    this.onDeleteBloak = null;
    this.onGetAllBloaks = null;
}

BloakView.prototype.render = function render(bloakDataObjectModel) {
    this.domElement.innerHTML += '<div class="bloakContainer"><h3>' + bloakDataObjectModel.title + '</h3><p class="bloakContent">' + bloakDataObjectModel.body + '</p><label class="bloakAuthor">' + bloakDataObjectModel.userId + '</label><br /><label class="bloakDate">' + bloakDataObjectModel.id + '</label><br /><input id=' + bloakDataObjectModel.id + ' type="button" value="Delete Bloak" /></div>';

    var addBloakButton = document.getElementById('addBloakButton');

    var deleteBloakButton = document.getElementById(bloakDataObjectModel.id);

    addBloakButton.addEventListener('click', this.onAddBloak);

    deleteBloakButton.addEventListener('click', this.onDeleteBloak);
}

App.js

var bloakModel = new BloakModel(XMLHttpRequest);

var targetElement = document.getElementById('bloaksContainer');
var bloakView = new BloakView(targetElement);

var bloakController = new BloakController(bloakModel, bloakView);

bloakController.init();

bloakController.getAllBloaks();
oakar
  • 1,187
  • 2
  • 7
  • 21

2 Answers2

3

Update

As it turns out I was totally wrong, having done more research, looks like using the innerHTML property will cause for the destruction of all child elements, read more about it here. Either way, the solutions provided will still work, turns out I didn't know that myself.

My Brain Fart

I should've realised that I was wrong initially, if I was right then you'd have an error stating that you can't attach an event listener to null or undefined, my bad, stupid mistake on my part!


Idea

Having very briefly looked over your code, I think it's safe to say that your render function is somewhat faulty, due to using innerHTML, it's not parsing the updated HTML fast enough for the event listeners to work...

Having tested it myself, and having just done the following:

setTimeout(() => {
    var addBloakButton = document.getElementById('addBloakButton');
    var deleteBloakButton = document.getElementById(bloakDataObjectModel.id);
    addBloakButton.addEventListener('click', this.onAddBloak);
    deleteBloakButton.addEventListener('click', this.onDeleteBloak);
}, 10);

It worked. So, if you don't want to change too much, you could do what I've done above, or alternatively, you could do something along the lines of this:

  var div = document.createElement("div");
  div.innerHTML = '<div class="bloakContainer"><h3>' + bloakDataObjectModel.title + '</h3><p class="bloakContent">' + bloakDataObjectModel.body + '</p><label class="bloakAuthor">' + bloakDataObjectModel.userId + '</label><br /><label class="bloakDate">' + bloakDataObjectModel.id + '</label><br /><input id=' + bloakDataObjectModel.id + ' type="button" value="Delete Bloak" /></div>';
  this.domElement.append(div);

  var addBloakButton = document.getElementById('addBloakButton');
  var deleteBloakButton = div.querySelector('input[id*="' + bloakDataObjectModel.id + '"]');
  addBloakButton.addEventListener('click', this.onAddBloak);
  deleteBloakButton.addEventListener('click', this.onDeleteBloak);

Furthermore, what I would do as a more solid and reliable approach is to implement an onRender function within the view, essentially stating that once and only once all of the relevant HTML has been rendered, then fire the onRender method. Or maybe something like view.dispatchEvents, etc... Specifically within this example, I'd run such a function within the controller, so once getAllBloaksList has rendered all of the relevant data, you can then fire a dispatchEvents function, like so:

// Within the controller part... 
BloakController.prototype.getAllBloaksList = function getAllBloaksList(allBloaksList) {
  for (var i = 0; i < allBloaksList.length; i++) {
    var bloak = allBloaksList[i];
    var bloakDataObjectModel = {
      title: bloak.title,
      body: bloak.body,
      userId: bloak.userId,
      id: bloak.id
    };

    this.view.render(bloakDataObjectModel);
  }

  this.view.dispatchEvents();
};


// .. Within the View part...
BloakView.prototype.dispatchEvents = function () {
  var list = this.domElement.querySelectorAll(".bloakContainer");
  var todo = this.onDeleteBloak;

  list.forEach(function (div) {
    var deleteButton = div.querySelector("input[type=button]");

    deleteButton.onclick =  function () {
      todo(this.id);
    };
  });
};

Demo

// Controller.
var BloakController = function BloakController(model, view) {
  this.model = model;
  this.view = view;
};

BloakController.prototype.init = function init() {
  this.view.onAddBloak = this.addBloak.bind(this);
  this.view.onDeleteBloak = this.deleteBloak.bind(this);
  this.view.onGetAllBloaks = this.getAllBloaks.bind(this);
};

BloakController.prototype.addBloak = function addBloak() {
  // todo
};

BloakController.prototype.getAllBloaks = function getAllBloaks() {
  this.model.getAllBloaks(this.getAllBloaksList.bind(this));
};

BloakController.prototype.getAllBloaksList = function getAllBloaksList(allBloaksList) {
  for (var i = 0; i < allBloaksList.length; i++) {
    var bloak = allBloaksList[i];
    var bloakDataObjectModel = {
      title: bloak.title,
      body: bloak.body,
      userId: bloak.userId,
      id: bloak.id
    };
    this.view.render(bloakDataObjectModel);
  }
  this.view.dispatchEvents();
};

BloakController.prototype.deleteBloak = function deleteBloak(id) {
  this.model.deleteBloak(id, this.getAllBloaksList.bind(this));
};


// Model.
var BloakModel = function BloakModel(XMLHttpRequest) {
  this.XMLHttpRequest = XMLHttpRequest;
};

BloakController.prototype.addBloak = function addBloak() {
  // todo
};

BloakModel.prototype.getAllBloaks = function getAllBloaks(callback) {
  var request = new this.XMLHttpRequest;
  request.onload = function onLoad(e) {
    var ajaxResponse = JSON.parse(e.currentTarget.responseText);
    callback(ajaxResponse);
  }
  request.open('GET', 'https://jsonplaceholder.typicode.com/posts', true);
  request.send();
};

BloakModel.prototype.deleteBloak = function deleteBloak(id, callback) {
  console.log(id);
  var request = new this.XMLHttpRequest;
  request.onload = function onLoad(e) {
    var ajaxResponse = JSON.parse(e.currentTarget.responseText);
    callback(ajaxResponse);
  }
  request.open('DELETE', 'https://jsonplaceholder.typicode.com/posts/' + id, true);
  request.send();
};


// View. 
var BloakView = function BloakView(domElement) {
  this.domElement = domElement;
  this.onAddBloak = null;
  this.onDeleteBloak = null;
  this.onGetAllBloaks = null;
};

BloakView.prototype.render = function render(bloakDataObjectModel) {
  this.domElement.innerHTML += '<div class="bloakContainer"><h3>' + bloakDataObjectModel.title + '</h3><p class="bloakContent">' + bloakDataObjectModel.body + '</p><label class="bloakAuthor">' + bloakDataObjectModel.userId + '</label><br /><label class="bloakDate">' + bloakDataObjectModel.id + '</label><br /><input id=' + bloakDataObjectModel.id + ' type="button" value="Delete Bloak" /></div>';
}

BloakView.prototype.dispatchEvents = function() {
  var list = this.domElement.querySelectorAll(".bloakContainer");
  var todo = this.onDeleteBloak;
  var addBloakButton = document.getElementById('addBloakButton');
  addBloakButton.addEventListener('click', this.onAddBloak);

  list.forEach(function(div) {
    var deleteButton = div.querySelector("input[type=button]");
    deleteButton.onclick = function() {
      todo(this.id);
    };
  });
};


// App.
var bloakModel = new BloakModel(XMLHttpRequest);
var targetElement = document.getElementById('bloaksContainer');
var bloakView = new BloakView(targetElement);
var bloakController = new BloakController(bloakModel, bloakView);

bloakController.init();
bloakController.getAllBloaks();
<button id="addBloakButton">Add</button>
<div id="bloaksContainer">
  <!-- DHTML -->
</div>
JO3-W3B-D3V
  • 2,124
  • 11
  • 30
  • 1
    thank you for your time. This really helped me :) I did not expect this kind of fault :/ It is really hard to catch for me and I spent a lots of time with that :/ – oakar Feb 04 '19 at 17:47
  • 1
    It's my pleasure! Having learned JavaScript the hard way, I've been exposed to all sorts of bugs similar to this. Also due to side effects, that's why I've personally begun to implement my JavaScript code with a more functional approach... Sorry for the short reply, I'm currently out and about, I'm typing this on my phone and the touch screen is somewhat broken, haha. – JO3-W3B-D3V Feb 04 '19 at 17:50
  • I've just updated my answer, I just learned the real reason as to why it wasn't working... Either way, the solution I've provided should still work! :) – JO3-W3B-D3V Feb 04 '19 at 18:03
  • 1
    Thank you again, it is really good to know there is something called "destruction of all child elements" when using innerHTML. :) – oakar Feb 04 '19 at 18:07
  • @oakar It _essentially_ just translates to the DOM elements are being destroyed, if you want more detail, I'd just look into it, I'm sure there's mention of this on MDN somewhere. – JO3-W3B-D3V Feb 04 '19 at 18:25
  • 1
    This answer also explains a little bit and makes sense I guess, see the accepted answer, https://stackoverflow.com/questions/5113105/manipulating-innerhtml-removes-the-event-handler-of-a-child-element – oakar Feb 04 '19 at 18:27
  • 1
    @oakar That is probably the most concise way in which I could think to explain it! :) – JO3-W3B-D3V Feb 04 '19 at 18:28
1

The code this.domElement.innerHTML += ...; is incorrect.You are using a += operator which adds something with itself and then assign the new value.

The new value is the result of existing innerHTML and new HTML. So the new value [HTML] will not have the previously added event handlers, it will just copy the HTML.

You can use appendChild method to append new elements.

Also, you should avoid using the for-loop with innerHTML because it will keep on updating DOM and you may see flickers on the screen. If you must use innerHTML, you should

  1. Create the entire HTML [innerHTML] together and inject at once,
  2. Then use event delegation to add event handlers.

Event delegation is a technique to bind events to a top-level container so that you do not need to worry about attaching the event listener to each item.

You can read more on DOM here.

Jayendra Sharan
  • 1,043
  • 6
  • 10