javascript dynamically adding classes without using inline functions

Question

I am working on a simple example, if a user clicks on element then all the elements above it should have a class and all elements below it should not have any class applied to them.

Here is my code:

<style>
p {
  background-color: skyblue;
  text-align: center;
  color: white;
  width: 50px;
  height: 50px;
  cursor: pointer;
}
.active {
  color: red;
  background-color: black;
}

</style>

<div id="divid">
  <p id="p1">one</p>
  <p id="p2">two</p>
  <p id="p3">three</p>
  <p id="p4">four</p>
</div>
<script>

function myFunction(index) {
    for (let i = 0; i <= index; i++) {
        paragraphs[i].classList.add('active');
    }
    for (let i = index; i < paragraphs.length; i++) {
        paragraphs[i].classList.remove('active');
    }
}

var paragraphs = document.querySelectorAll("p");
console.log(paragraphs);
for(var j=0;j<paragraphs.length; j++) {
    paragraphs[j].addEventListener("click", myFunction(j));
}

</script>

When I run this code it is directly setting the class active to first 3 paragraph tags which is not the expected behaviour, and also the click function is not working.

If I replace the javascrip code with inline function, it is working without issues.

var paragraphs = document.querySelectorAll("p");
console.log(paragraphs);
for(var j=0;j<paragraphs.length; j++) {
    paragraphs[j].addEventListener("click", (function(index) {
    return function() {
         for (let i = 0; i <= index; i++) {
            paragraphs[i].classList.add('active');
        }
        for (let i = index; i < paragraphs.length; i++) {
            paragraphs[i].classList.remove('active');
        }
    }
  })(j));
}

Can you please help me what is the issue with my code if I place it as external function?

with inline function What do you mean? The second snippet doesn't look like an inline function, it looks like a standard script, which there's nothing wrong with. It is in the Javascript and not the HTML, right? — CertainPerformance
– CertainPerformance, Commented Jun 24, 2018 at 4:20

brk · Accepted Answer · 2018-06-24 04:36:01Z

myFunction(j) in addEventListener will immediately execute the function. Replace it with only myFunction. The myFunction will get the event object. From event object you can get the target which will be useful to get the index of the child element.

You can adjust the iteration over m value to add or remove class from desired elements

function myFunction(index) {
  // this will first create an array of all the child elmenets
  // then the indexOf will get the index of the child which was clicked
  let m = Array.from(index.target.parentNode.children).indexOf(index.target);
  for (let i = 0; i < m; i++) {
    paragraphs[i].classList.add('active');
  }
  for (let i = m + 1; i < paragraphs.length; i++) {
    paragraphs[i].classList.remove('active');
  }
}

var paragraphs = document.querySelectorAll("p");

for (var j = 0; j < paragraphs.length; j++) {
  paragraphs[j].addEventListener("click", myFunction);
}

p {
  background-color: skyblue;
  text-align: center;
  color: white;
  width: 50px;
  height: 50px;
  cursor: pointer;
}

.active {
  color: red;
  background-color: black;
}

<div id="divid">
  <p id="p1">one</p>
  <p id="p2">two</p>
  <p id="p3">three</p>
  <p id="p4">four</p>
</div>

Jake Miller · Accepted Answer · 2018-06-24 04:23:53Z

1

I'm suspecting the reason you're encountering an issue is because typically external files are loaded before all of your DOM elements get a chance to load. Whenever you have an inline function, typically you define it after your DOM elements so your elements have loaded which is why you're able to select them and add event listeners.

Try wrapping your logic in this:

document.addEventListener("DOMContentLoaded", function(event) {
   // Place your code here.
});

Or if you use jQuery, use:

$(document).ready(function() {
  // Place your code here.
});

This'll allow your DOM elements to load before you attempt to use them in your code making them accessible in separate js files.

answered Jun 24, 2018 at 4:23

Jake Miller

2,6622 gold badges27 silver badges41 bronze badges

Comments

Mamun · Accepted Answer · 2018-06-24 05:02:11Z

You can call the function inside of an anonymous function:

function myFunction(index) {
  for (let i = 0; i < index; i++) {
    paragraphs[i].classList.add('active');
  }
  for (let i = index; i < paragraphs.length; i++) {
    paragraphs[i].classList.remove('active');
  }
}

var paragraphs = document.querySelectorAll("p");

for(let j=0;j<paragraphs.length; j++) {
  paragraphs[j].addEventListener("click", function(){
    myFunction(j);
  });
}

p {
  background-color: skyblue;
  text-align: center;
  color: white;
  width: 50px;
  height: 50px;
  cursor: pointer;
}
.active {
  color: red;
  background-color: black;
}

<div id="divid">
  <p id="p1">one</p>
  <p id="p2">two</p>
  <p id="p3">three</p>
  <p id="p4">four</p>
</div>

Community · Accepted Answer · 2020-06-20 09:12:55Z

The following uses Event Delegation by having the parent div listen for the click event for all of its descendant nodes. Now any <p> clicked would be the event.target and the parent div is event.currentTarget. Those 2 Event properties will reliably reference the nodes without the need for ids, classes, attributes or tagNames.

As an external file place the <script> tag before the closing </body> tag. It works 99.9% of the time.

Plunker

Demo

Details commented in Demo

// Reference the parent node
var mom = document.querySelector('div');

/* Collect all of mom's paragraphs into a nodeList then turn it into an array.
 */
var kids = Array.from(mom.querySelectorAll('p'));

/* Event Delegation
|| Register the click event to mom
|| Now any click to a kid (e.target) will propagate to mom (e.currentTarget). 
|| That's only one event listener for any number of descendant nodes.
*/
mom.addEventListener('click', olderBros);


function olderBros(e) {

  // Remove .active from all of the kids
  kids.forEach(function(k) {
    k.className = "";
  });

  // Make sure that the clicked node is NOT mom 
  if (e.target !== e.currentTarget) {

    // Reference the clicked node...
    var tgt = e.target;

    // Assign the clicked node .active class
    // Remove this line if you don't want clicked node .active
    tgt.className = "active";

    var i, j = kids.legnth;
    // Add .active to all older/previous <p>/Element Bros/Sibling
    for (i = 0; tgt = tgt.previousElementSibling; i++) {
      kids[i].classList.add('active');
    }

    // Remove .active to all younger/next <p>/Element Bros/Sibling
    for (let k = (i + 1); k < (j - i); k++) {
      kids[k].classList.remove('active');
    }

  } else {
    return false;
  }
  // Stop event bubbling
  e.stopPropagation()
}

<!DOCTYPE html>
<html>

<head>
  <style>
    p {
      background-color: skyblue;
      text-align: center;
      color: white;
      width: 50px;
      height: 50px;
      cursor: pointer;
    }
    
    .active {
      color: red;
      background-color: black;
    }
  </style>
</head>

<body>
  <div>
    <p>one</p>
    <p>two</p>
    <p>three</p>
    <p>four</p>
    <p>five</p>
    <p>six</p>
    <p>seven</p>
    <p>eight</p>
    <p>nine</p>
    <p>ten</p>
    <p>eleven</p>
    <p>twelve</p>
  </div>

  <script src="script.js"></script>

</body>

</html>

Collectives™ on Stack Overflow

javascript dynamically adding classes without using inline functions

4 Answers 4

Comments

Comments

Comments

Plunker

Demo

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Demo

Comments

Your Answer

Sign up or log in

Post as a guest

Related