CODEWITHSUNDEEP
javascripthtml
1878castle
1878castle

Reputation: 61

Cloning script doesn't work in vanilla javascript

I cannot find the error in my code. This is supposed to be a simple clone and remove script. I am trying to convert it from a Jquery script, but no luck so far.

document.querySelector(".wrapper").addEventListener("click", ".remove", function() {
  document.querySelector(".remove")
    .closest(".wrapper")
    .querySelector(".element")
    .not(":first")
    .last()
    .remove();
});
document.querySelector(".wrapper").addEventListener("click", ".clone", function() {
  document.querySelector(".clone")
    .closest(".wrapper")
    .querySelector(".element")
    .first()
    .clone()
    .appendTo(".results");
});
body {
  padding: 1em;
}

.element {
  background: #eee;
  width: 200px;
  height: 40px;
  padding: 20px 20px 0;
  text-align: center;
  margin: 5px 0;
}

.buttons {
  clear: both;
  margin-top: 10px;
}
<div class="wrapper">
  <div class="element"></div>
  <div class="results"></div>

  <div class="buttons">
    <button class="clone">clone</button>
    <button class="remove">remove</button>
  </div>
</div>

Upvotes: 0

Views: 126

Answers (1)

pilchard
pilchard

Reputation: 12920

There are a number of problems with you code, but the main error is that you are passing the wrong arguments to addEventListener whose syntax is target.addEventListener(type, listener [, options]); where type is the event type and listener is a callback function. The callback will be passed the event object intrinsically.

Since you are attaching the listener to the wrapper element, you need to evaluate the event.target when triggered to determine which button was actually clicked.

if (event.target.classList.contains('clone')) {...

You can then call the necessary function based on this evalution passing either the entire event object, or in the example below, just the element returned by event.target.

Next, while chaining will get you a long way, it is actually advantageous here to declare some variables to avoid multiple queries to the same element.

A few smaller things :

It isn't necessary to check for first as querySelector only returns the first element that matches.

Likewise, specifying not(:first) won't stop querySelector as it only returns the first.

In order to access the last element, you can simply get them all using querySelectorAll and retrieve the last element from the returned NodeList. (destructured to an array here)

[...allElements][allElements.length-1].remove();


function clone(button) {
  const wrapper = button.closest(".wrapper");
  const cloneElement = wrapper.querySelector(".element").cloneNode(true);
  wrapper.querySelector(".results").appendChild(cloneElement);
}

function remove(button) {
  const allElements = button.closest(".wrapper").querySelectorAll(".element");
  if (allElements.length >1) [...allElements][allElements.length-1].remove();
}

document.querySelector(".wrapper").addEventListener("click", function (event) {
  if (event.target.classList.contains('clone')) {
    clone(event.target);
  } else if (event.target.classList.contains('remove')) {
    remove(event.target);
  }
});
body {
  padding: 1em;
}
.element {
  background: #eee;
  width: 200px;
  height: 40px;
  padding: 20px 20px 0;
  text-align: center;
  margin: 5px 0;
}
.results .element {
  background-color: aquamarine;
}
.buttons {
  clear: both;
  margin-top: 10px;
}
<div class="wrapper">
  <div class="element">
  Element
  </div>
  <div class="results"></div>

  <div class="buttons">
    <button class="clone">clone</button>
    <button class="remove">remove</button>
  </div>
</div>

Upvotes: 1

Related Questions