0

I want to add active class using below function:

function setActive() {
  aObj = document.getElementById('pagesidebar2').getElementsByTagName('a');
  for(i=0;i<aObj.length;i++) {
    if(document.location.href.indexOf(aObj[i].href)>=0) {
      aObj[i].className='active';
    }
  }
}

window.onload = setActive;

This function is only applicable to a list within pagesidebar2 div.

<div id="pagesidebar2">
  <ul>
    <li><a href="home.html">Home</a></li>
    <li><a href="about.html">About</a></li>
    <li><a href="blog.html">Blog</a></li>
    <li><a href="contact.html">Contact</a></li>
  </ul>
</div>

Let's say I want to use the same function to add active class to:

<div id="pagesidebar3">
  <ul>
    <li><a href="#">uae</a></li>
    <li><a href="#">ASIA</a></li>
    <li><a href="#">AFRICA</a></li>
    <li><a href="#">Europe</a></li>
  </ul>
</div>

What changes should i make to the setActive() function?

veebo94
  • 3
  • 1

5 Answers5

0

You could have the navigation id, as an argument to your setActive function

function setActive(nav) {
  aObj = document.getElementById(nav).getElementsByTagName('a');
  for(i=0;i<aObj.length;i++) {
    if(document.location.href.indexOf(aObj[i].href)>=0) {
      aObj[i].className='active';
    }
  }
}
onload = ()=>{setActive('pagesidebar2'); setActive('pagesidebar3')}
kess
  • 1,204
  • 8
  • 19
0

One solution would be to use an array of things you want to add the class to, and iterate over it. For instance, to do sidebars 2 and 3:

const idsOfThingsToSetActive = ['pagesidebar2', 'pagesidebar3'];
function setActive() {
  idsOfThingsToSetActive.forEach(id => {
    aObj = document.getElementById(id).getElementsByTagName('a');
    for(i=0;i<aObj.length;i++) {
      if(document.location.href.indexOf(aObj[i].href)>=0) {
        aObj[i].className='active';
      }
    }
  });
}

Another would be to parameterize (ie. add an argument to) your function, then call your function once for each thing you want to set active; @Gbr22 gave a good example of that in their answer.

Which approach you take is really a human concern: do you want a function that focuses several things, or a function that focuses one thing, and you use it multiple times?

The rest of your code might impact this: for instance, if you need to focus one or multiple elements elsewhere, besides onload, then you probably want to do things the same in both places so that you can re-use the function.

machineghost
  • 33,529
  • 30
  • 159
  • 234
0

Rather than hard-code the div id, you would pass a reference to the div that you need to work on as an argument to the function.

Also, take a look at this re-worked syntax. getElementsByTagName() is inefficient and even more so when used within a loop. You should avoid using it and instead use .querySelectorAll(). You can then directly loop over the resulting collection with .forEach(), which eliminates the need for loop indexes.

And, are you sure you want your indexOf check to compare against 0? indexOf returns -1 if an item can't be found. 0 is a valid answer that means the searched on element was present.

function setActive(div) {
  // Find all the <a> elements within the passed div reference and loop over them
  div.querySelectorAll("a").forEach(function(a){
    if(location.href.indexOf(a.href)>=0) {
      a.className='active';
    }
  }):
}

window.onload = setActive(document.getElementById("pagesidebar2"));

// Then any other time you want to call the function, just pass the appropriate div
setActive(document.getElementById("pagesidear3"));
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
0

You can use a class instead of an ID. This is a great option as you won't have to modify the javascript every time you need to loop through another set of links, just give the parent a class of "sidebar".

function setActive() {
var _url = "https://stacksnippets.net/home.html";//document.location.href
  aObj = document.querySelectorAll('.sidebar a');
  for(i=0;i<aObj.length;i++) {
    if(_url.indexOf(aObj[i].href)>=0) {
      aObj[i].className='active';
    }
  }
}

setActive();
.active{color:red;}
<div class="sidebar" id="pagesidebar2">
  <ul>
    <li><a href="home.html">Home</a></li>
    <li><a href="about.html">About</a></li>
    <li><a href="blog.html">Blog</a></li>
    <li><a href="contact.html">Contact</a></li>
  </ul>
</div>

<div class="sidebar" id="pagesidebar3">
  <ul>
    <li><a href="#">uae</a></li>
    <li><a href="#">ASIA</a></li>
    <li><a href="#">AFRICA</a></li>
    <li><a href="#">Europe</a></li>
  </ul>
</div>
imvain2
  • 15,480
  • 1
  • 16
  • 21
0

try querySelectorAll

function setActive() {
  let aObj = document.querySelectorAll('#pagesidebar2,#pagesidebar3  a');
  console.log(aObj);
  for(i=0;i<aObj.length;i++) {
    if(document.location.href.indexOf(aObj[i].href)>=0) {
      aObj[i].className='active';
    }
  }
}

window.onload = setActive;
.active{ color:'red'}
<div id="pagesidebar2">
  <ul>
    <li><a href="home.html">Home</a></li>
    <li><a href="about.html">About</a></li>
    <li><a href="blog.html">Blog</a></li>
    <li><a href="contact.html">Contact</a></li>
  </ul>
</div>
<div id="pagesidebar3">
  <ul>
    <li><a href="#">uae</a></li>
    <li><a href="#">ASIA</a></li>
    <li><a href="#">AFRICA</a></li>
    <li><a href="#">Europe</a></li>
  </ul>
</div>
Mohammad Ali Rony
  • 4,695
  • 3
  • 19
  • 33