0

I am practising event delegation and I stumbled upon this. I have 4 paragraphs setting inside an article tag which sits in a div. I want to apply a .green class that changes background of each paragraph ONLY when I click on it. However the green class gets only applied up to the parent (article) and the grandparent (div) as well. Tell me what's wrong with my code please.

function paintParagraph(e) {
    if (!e) {
        e = window.event;
    }
    var target, parent;
    target = e.target || e.srcElement;
    parent = target.parentNode;
    parent.className = 'green';
    e.stopPropagation();
    
}

var paragraphList = document.querySelector('#p-list');
paragraphList.addEventListener('click', function(e) {

paintParagraph(e);

}
, false);
    p {
        padding: 10px;
        margin: 20px;
        border: 2px solid;
        line-height: 1.5em;
        letter-spacing: 0.2em;
        text-align: center;
    }

  .green {
        background-color: green;
    }
<div id="main">
        <article id="p-list">
            <p class="para">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Vitae perferendis culpa velit excepturi iste reprehenderit quaerat debitis repudiandae itaque beatae. Numquam dolorum ducimus iusto temporibus facilis error possimus amet, illo.</p
            ><p class="para2">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Vitae perferendis culpa velit excepturi iste reprehenderit quaerat debitis repudiandae itaque beatae. Numquam dolorum ducimus iusto temporibus facilis error possimus amet, illo.</p
            ><p class="para3">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Vitae perferendis culpa velit excepturi iste reprehenderit quaerat debitis repudiandae itaque beatae. Numquam dolorum ducimus iusto temporibus facilis error possimus amet, illo.</p
            ><p class="para4">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Vitae perferendis culpa velit excepturi iste reprehenderit quaerat debitis repudiandae itaque beatae. Numquam dolorum ducimus iusto temporibus facilis error possimus amet, illo.</p
        ></article>
    </div>
Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
Ahmed Magdy
  • 331
  • 1
  • 3
  • 9
  • Use `target.className = 'green';` – Rayon Jan 06 '17 at 17:47
  • I tried that before I post this question but it still applies class green to the paragraph tag and its containing article tag as well. how can I prevent it affecting the article tag? – Ahmed Magdy Jan 06 '17 at 17:50
  • Because you are applying it to `target.parentNode` as well.. `parentNode` is nothing but father of the current node.. – Rayon Jan 06 '17 at 17:51
  • Not exactly related, but the check for `e` fails in `paintParagraph`. The global `event` object is available in the handler function only, though you've `window.object`. You've to do the check in the handler, and pass the `event` object to `paintParagraph`. – Teemu Jan 06 '17 at 17:51
  • @Rayon well actually no I edited the code to be target.className = 'green' and removed the parentNode code and the parent variable but it still applies to the paragraph and the article containing tag. – Ahmed Magdy Jan 06 '17 at 17:52

2 Answers2

1

You should check if the clicked target is p then only proceed further.

if(target.nodeName !== "P") return false;

Now you will have only p element as the target apply the class on it.

 target.className = 'green';

function color(elem) {
  if (elem.nodeName == "P") {
    elem.className = 'green';
  }

  if (elem.parentNode !== null) {
    color(elem.parentNode)
  }
}



function paintParagraph(e) {
  if (!e) {
    e = window.object;
  }
  var target, parent;
  target = e.target || e.srcElement;
  color(target)

}

var paragraphList = document.querySelector('#p-list');
paragraphList.addEventListener('click', function(e) {

  paintParagraph(e);

}, false);
p {
  padding: 10px;
  margin: 20px;
  border: 2px solid;
  line-height: 1.5em;
  letter-spacing: 0.2em;
  text-align: center;
}
.green {
  background-color: green;
}
<div id="main">
  <article id="p-list">
    <p class="para">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Vitae perferendis culpa velit excepturi iste reprehenderit quaerat debitis repudiandae itaque beatae. Numquam dolorum ducimus iusto temporibus facilis error possimus amet, illo.</p>
    <p class="para2">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Vitae perferendis culpa velit excepturi iste reprehenderit quaerat debitis repudiandae itaque beatae. Numquam dolorum ducimus iusto temporibus facilis error possimus amet, illo.</p>
    <p class="para3">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Vitae perferendis culpa velit excepturi iste reprehenderit quaerat debitis repudiandae itaque beatae. Numquam dolorum ducimus iusto temporibus facilis error possimus amet, illo.</p>
    <p class="para4">Lorem ipsum dolor sit amet, consectetur adipisicing elit. Vitae perferendis culpa <span> HELLO</span>velit excepturi iste reprehenderit quaerat debitis repudiandae itaque beatae. Numquam dolorum ducimus iusto temporibus facilis error possimus amet,
      illo.</p>
  </article>
</div>
Deep
  • 9,594
  • 2
  • 21
  • 33
  • Thank you for your answer. But is checking the nodeName a must? is there another workaround? – Ahmed Magdy Jan 06 '17 at 17:54
  • 1
    What if `p` contains `p` childs ? – Rayon Jan 06 '17 at 17:54
  • Exactly. How can I put the event.stopPropagation(); method in use? – Ahmed Magdy Jan 06 '17 at 17:55
  • @AhmedMagdy this is a safety check that the if user has clicked on the paragraph only then add the class. – Deep Jan 06 '17 at 17:56
  • @Rayon Yes agree, this solution need to be modified if the html is different than what provided here – Deep Jan 06 '17 at 17:57
  • And what is the modification going to be like? – Ahmed Magdy Jan 06 '17 at 17:58
  • @AhmedMagdy some recursion may help, you can keep checking for parent node and add class if its a p element. – Deep Jan 06 '17 at 18:08
  • @Rayon what do you say? – Deep Jan 06 '17 at 18:08
  • @Deep I want to use event.stopPropagation(); – Ahmed Magdy Jan 06 '17 at 18:13
  • @AhmedMagdy There is no propagation of event here, your class was applied on the parent element since you applied on it. It only depends upon where in the html you have clicked, if you have clicked a paragraph element, class will applied on article element , if you have clicked outside of paragraph i.e. the article element then class will be applied on its parent i.e. div – Deep Jan 06 '17 at 18:16
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/132524/discussion-between-ahmed-magdy-and-deep). – Ahmed Magdy Jan 06 '17 at 18:19
0

As far as I understand you want the <article> element to have class .green only when user click <p> element if this is true, then this is my answer.

In this case I reference the <p> elements instead of the <article> element.

function paintParagraph(e) {
  if (!e) {
    e = window.event;
  }
  const { target, srcElement } = e;
  parent = target.parentNode;
  parent.className = 'green';
  e.stopPropagation();
}

var paragraphList = document.querySelectorAll('#p-list > p');
paragraphList.forEach((element) => {
  element.addEventListener('click', paintParagraph, false);
});
German
  • 1,126
  • 8
  • 20