6

I've built a simple demo (https://codepen.io/anon/pen/VgKQoq) of elements and objects being created when clicking the button: it creates an element and an object of that element is then pushed into an 'objects' array. When clicking the remove button, the element and object are successfully removed by using ids.

However, the problems is that each time an element is removed, the remove function sometimes runs too many times depending on which element is clicked, and I don't know why. On the demo, open the javascript console, create for example 4 elements, and then remove the 3rd element by clicking remove, and you'll see what happens.

Does anyone know why this happens? I thought it may be because the event listener is added again and again multiple times to the same elements, but it seemingly doesn't work when removed. Any explanations and best practices would be appreciated here, thank you.

var id = 0, objects = [], removes;

function createEntry() {
	id++;

	// create new element, append to #container & create new object
	var container = document.querySelector('#container'),
	    newEntry = document.createElement("div"),
	    title = 'title text here',
	    description = 'description text here',
	    remove = 'remove',
	    dataId = id,
	    obj = new Entry(title, description, remove);
	newEntry.classList.add("entry");
	newEntry.innerHTML = '<span class="title">' + title + '</span><span class="description">' + description + '</span><span class="remove">' + remove + '</span>';
	container.appendChild(newEntry);
	newEntry.setAttribute('data-id', id);

	updateElements();

	// constructor & push into array
	function Entry(title, description, remove) {
		this.title = title;
		this.description = description;
		this.remove = remove;
		this.id = id;

		objects.push(this);
	}

	// tests
	console.log('JSON.stringify(obj): ' + JSON.stringify(obj));
	console.log('obj.id: ' + obj.id);

	function updateElements() {
		removes = document.querySelectorAll(".remove");
		listenForRemoves();

		function listenForRemoves() {
			for (let remove of removes) {
				remove.removeEventListener("click", removeElements);
				remove.addEventListener("click", removeElements);
			}
		}

		function removeElements() {
			let removedId = this.parentNode.getAttribute('data-id'),
			    objToRemove = objects.find(obj => obj.id == removedId); // not used

			this.parentNode.remove(); console.log('removed id ' + removedId);
				console.log('objects before: '); for (let object of objects) { console.log(JSON.stringify(object))};
				objects = objects.filter(obj => obj.id != removedId); // doesn't use objToRemove
				console.log('objects now: '); for (let object of objects) { console.log(JSON.stringify(object))};
		}
	}
	// works but why the repeating console logs twice?
}
button { display: block }
.entry {
	width: 100%;
	display: block;
	padding: 10px;
	border: 1px solid #f5f5f5;
}
span {
  display: block;
  width: 100%;
}
section { background: lightgreen }
<button id='btn' onclick='createEntry()'>Create</button>
<section id='container'></section>

UPDATE: Any other ideas? I've added remove.removeEventListener("click", removeElements); which now gets rid of a lot of duplicates, but it now still console logs only twice (well... sometimes!?). New codepen link updated above

3
  • 1
    because you bind too many events in loop even if it's not needed
    – dfsq
    Commented Jan 29, 2019 at 12:47
  • 2
    You are adding a new event listener to the same element every time something is updated, without removing previous event listeners. Commented Jan 29, 2019 at 12:49
  • I've added 'remove.removeEventListener("click", removeElements)' which has removed a lot of the duplicate console logging, but now the console logs twice. Does anyone have any ideas? I'm out of ideas and have tried lots of changes Commented Jan 29, 2019 at 15:28

1 Answer 1

1

I'm not sure exactly what's happening but you have all of these functions nested inside of the createEntry function. Try moving them outside of that function. This seems to solve the problem on my tests:

var id = 0, objects = [], removes;

function createEntry() {
    id++;

    // create new element, append to #container & create new object
    var container = document.querySelector('#container'),
        newEntry = document.createElement("div"),
        title = 'title text here',
        description = 'description text here',
        remove = 'remove',
        dataId = id,
        obj = new Entry(title, description, remove);
    newEntry.classList.add("entry");
    newEntry.innerHTML = '<span class="title">' + title + '</span><span class="description">' + description + '</span><span class="remove">' + remove + '</span>';
    container.appendChild(newEntry);
    newEntry.setAttribute('data-id', id);

    updateElements();

    // constructor & push into array
    function Entry(title, description, remove) {
        this.title = title;
        this.description = description;
        this.remove = remove;
        this.id = id;

        objects.push(this);
    }

    // tests
    console.log('JSON.stringify(obj): ' + JSON.stringify(obj));
    console.log('obj.id: ' + obj.id);
}

function updateElements() {
  removes = document.querySelectorAll(".remove");
  listenForRemoves();

  function listenForRemoves() {
    for (let remove of removes) {
      remove.removeEventListener("click", removeElements);
      remove.addEventListener("click", removeElements);
    }
  }


}
function removeElements(e) {
  let removedId = this.parentNode.getAttribute('data-id'),
      objToRemove = objects.find(obj => obj.id == removedId); // not used

  this.parentNode.remove(); console.log('removed id ' + removedId);
    console.log('objects before: '); for (let object of objects) { console.log(JSON.stringify(object) + " " + e.target)};
    objects = objects.filter(obj => obj.id != removedId); // doesn't use objToRemove
    console.log('objects now: '); for (let object of objects) { console.log(JSON.stringify(object))};
}
1
  • thank you - not sure how the nested functions caused this error, but following your advice I also edited the original again and it worked by removing nested functions Commented Jan 30, 2019 at 8:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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