1

My goal is to delete and/or move the parent element of the button on the page when it is clicked. The DOM doesn't seem to respond to my click event. I think it has something to do with scope and global variables but I'm still a newbie. Here's what my code looks like.

var startButton = document.querySelectorAll('button[data-action="start"]');
var deleteButton = document.querySelectorAll('button[data-action="delete"]');
var element = document.getElementsByTagName('section');
var firstChildElement = element.firstChild;

startButton.addEventListener("click", toStart);
deleteButton.addEventListener("click", deleter);

/*function declarations*/
function toStart() {
  element.insertBefore(this.parentNode, firstChildElement);
}

function deleter() {
  element.removeChild(this.parentNode);
}
<section>
  <article>
    <h2>Item One </h2>
    <p>Lorem ipsum dolor sit amet</p>
    <button data-action="start">Move to start</button>
    <button data-action="delete">Delete</button>
  </article>
  <article>
    <h2>Item Two </h2>
    <p>Lorem ipsum dolor sit amet</p>
    <button data-action="start">Move to start</button>
    <button data-action="delete">Delete</button>
  </article>
  <article>
    <h2>Item Three </h2>
    <p>Lorem ipsum dolor sit amet</p>
    <button data-action="start">Move to start</button>
    <button data-action="delete">Delete</button>
  </article>
</section>
canon
  • 40,609
  • 10
  • 73
  • 97
mtkguy
  • 277
  • 2
  • 14
  • I don't see where you are importing your `.js` file in your view. But regardless of that, the answer below addresses the issue if you have successfully imported the javascript file into your view. – Ryan Wilson Sep 04 '18 at 20:03
  • 2
    Possible duplicate of [What do querySelectorAll, getElementsByClassName and other getElementsBy\* methods return?](https://stackoverflow.com/questions/10693845/what-do-queryselectorall-getelementsbyclassname-and-other-getelementsby-method) – Sebastian Simon Sep 04 '18 at 20:04

1 Answers1

3

That's because you are using querySelectorAll(), which returns a node list (a set of nodes) and you can't add an event listener (call .addEventListener()) on a set. You can only do it on an individual node. The same is true for .getElementsByTagName(). It also returns a node list and you can't ask for the .firstChild of a node list. You can only call it on a node.

You can use .querySelector() instead to get just the first node that matches the selector.

Now, if you want to set up events on all the buttons, then you'd need to loop over the node list and call .addEventListener() for each one.

But, since you want the same listeners on all the buttons, it's more efficient and less code to use event delegation, whereby we allow events to "bubble" up to ancestor elements and intercept the event just once at that higher level element. Then, we can examine which element actually triggered the event at a lower level and act accordingly.

// We're going to let all click events within the section be handled at the section level
// instead of setting up the same handlers on multiple elements.
let sec = document.querySelector("section");
sec.addEventListener("click", handleClick);

// When the event bubbles up to the section, this function will be called
// and it will automatically receive a reference to the event that triggered it
function handleClick(event){
  // The .target property of the event object refers to the lower-level ojbect
  // that actually initiated the event (either a start or delete button in our case).
  // We're going to need the nearest <article> ancestor of the button that was clicked
  // The closest() method finds the nearest ancestor element that matches the selector
  let art = event.target.closest("article");

  // We can check what the source of the event was and act accordingly
  // Since you've used data-* attributes, use the dataset API to test them
  if(event.target.dataset.action === "start"){
    toStart(art);
  } else if(event.target.dataset.action === "delete") {
    deleter(art);
  }
}

/*function declarations*/
function toStart(element){
  element.parentNode.insertBefore(element, element.parentNode.firstChild);
}

function deleter(element){
  element.parentNode.removeChild(element);
}
<section>
  <article>
    <h2>Item One </h2>
    <p>Lorem ipsum dolor sit amet</p>
    <button data-action="start">Move to start</button>
    <button data-action="delete">Delete</button>
  </article>
  <article>
    <h2>Item Two </h2>
    <p>Lorem ipsum dolor sit amet</p>
    <button data-action="start">Move to start</button>
    <button data-action="delete">Delete</button>
  </article>
  <article>
    <h2>Item Three </h2>
    <p>Lorem ipsum dolor sit amet</p>
    <button data-action="start">Move to start</button>
    <button data-action="delete">Delete</button>
  </article>
</section>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • Using `querySelector` will definitely solve this problem. – Travis J Sep 04 '18 at 20:06
  • Still not working. In addition i want this click and response event on all button nodes and not just the first – mtkguy Sep 04 '18 at 20:14
  • @mtkguy See my updated answer that addresses the new information you've shared. – Scott Marcus Sep 04 '18 at 21:59
  • That's not how you do event delegation. Event delegation should only trigger actions when you match a perfect selector. Having a void else like here set to fire the deleter means that if you click anywhere in one of these articles you'll delete it. – Kaiido Sep 05 '18 at 22:44
  • @Kaiido While you are correct that I left a bug in the `if` statement, you are incorrect that it has anything to do with what event delegation is. It is perfectly reasonable to have a use case where a default action may take place. `if` statement updated. Thanks for pointing that out. – Scott Marcus Sep 05 '18 at 22:58
  • @Kaiido Event delegation has nothing whatsoever to do with ***how*** to react to an event. It's about what element intercepts the event. It's about configuring events at a high level instead of configuring several at lower levels. As I said, it's perfectly reasonable to have a "default" action to perform unless the `event.target` matches a specific selector. That's not anarchy, it's simply a default handler. – Scott Marcus Sep 05 '18 at 23:03
  • OK, well you're going to have to agree to disagree because you certainly can have defaulted delegates and my lack of an `else` test actually proves that. I'd love for you to show my any kind of documentation that indicates otherwise. – Scott Marcus Sep 05 '18 at 23:17
  • Event delegation is not part of any official specs, it's coding practice. But you just have to read the [definition of delegation](https://www.merriam-webster.com/dictionary/delegation) to get my point. Which "others" your target is acting for in this default behavior. – Kaiido Sep 05 '18 at 23:22
  • That's right, it's not a spec. And, it's not even a practice. It's a common pattern that simply leverages bubbling. And that's why you can't put a strict set of implementation rules on it as you are. And, let's not resort to the dictionary, when we can do much better than that: *[Event delegation allows us to attach a single event listener, to a parent element, that will fire for all descendants matching a selector, whether those descendants exist now or are added in the future.](https://learn.jquery.com/events/event-delegation/)*... – Scott Marcus Sep 05 '18 at 23:28
  • ...And, extending that paradigm so that a default handler can fire doesn't mean that you aren't using event delegation, it just means you are extending the pattern. – Scott Marcus Sep 05 '18 at 23:29
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/179501/discussion-between-kaiido-and-scott-marcus). – Kaiido Sep 05 '18 at 23:31
  • Yeah, iterating over the returned nodelist solved the issue. Thanks @scott – mtkguy Sep 06 '18 at 06:41