I want the style to persist

The following javascript is used to click on a tab and style it with the active class. After it applies the style, it doesn’t persist. I want the tab to remain in the active state even after it has been clicked.

const tabs = document.getElementsByTagName("li");

for (const i = 0; i < tabs.length; i++) {
  tabs[i].addEventListener("click", function() {

  const current = document.getElementsByClassName("active");
  current[0].className = current[0].className.replace("active");
  this.className += "active";

  });
}

This is for an assignment so it has to be javascript and not jQuery.

You already have event listeners on the list items so you don’t need to do it all again. You can check for the active class in a function from each of your blocks.

e.g.

const liststart = document.querySelector("#navbar__list");
const listelement1 = document.createElement("li");
const listelement2 = document.createElement("li");
const listelement3 = document.createElement("li");
const listelement4 = document.createElement("li");

listelement1.classList.add("active");//set first item to active

listelement1.innerHTML = "TAB 1";
listelement2.innerHTML = "TAB 2";
listelement3.innerHTML = "TAB 3";
listelement4.innerHTML = "TAB 4";

const section1 = document.getElementById("section1");
const section2 = document.getElementById("section2");
const section3 = document.getElementById("section3");
const section4 = document.getElementById("section4");

listelement1.addEventListener("click", function () {
  findCurrent(this);
  section1.scrollIntoView(true);
  section1.classList.add("my-active");
  section2.classList.remove("my-active");
  section3.classList.remove("my-active");
  section4.classList.remove("my-active");
});

listelement2.addEventListener("click", function () {
  findCurrent(this);
  section2.scrollIntoView(true);
  section1.classList.remove("my-active");
  section2.classList.add("my-active");
  section3.classList.remove("my-active");
  section4.classList.remove("my-active");
});

listelement3.addEventListener("click", function () {
  findCurrent(this);
  section3.scrollIntoView(true);
  section1.classList.remove("my-active");
  section2.classList.remove("my-active");
  section3.classList.add("my-active");
  section4.classList.remove("my-active");
});

listelement4.addEventListener("click", function () {
  findCurrent(this);
  section4.scrollIntoView(true);
  section1.classList.remove("my-active");
  section2.classList.remove("my-active");
  section3.classList.remove("my-active");
  section4.classList.add("my-active");
});
liststart.appendChild(listelement1);
liststart.appendChild(listelement2);
liststart.appendChild(listelement3);
liststart.appendChild(listelement4);

function findCurrent(currentEl) {
  var current = liststart.getElementsByClassName("active");
  // should really check this exists first
  current[0].classList.remove("active");
  currentEl.classList.add("active");
}

That is a basic working example which will work in your codepen.

However as mentioned before that is not the correct way to do it as it is not scalable. You should be able to make the process of selecting a tab and finding the right section more automatic and not rely on hard coding everything and repeating the same routines over and over again. I assume you have been told to do it this way for your assignment but it is a very verbose way of doing something like this. :slight_smile:

Note that this is not valid html:

<ul id="navbar__list">
 <span><span>
<li>TAB 1</li><li>TAB 2</li><li>TAB 3</li><li>TAB 4</li>
</span></span>
</ul>

The spans are invalid in those positions which is why I removed then from the jS as they were doing nothing anyway.

You don’t really need js to scroll to a destination and style it as that can be done in CSS and html alone with fragment identifiers and using :target. However I guess you have been told to do this in JS so I won’t give an example. :slight_smile:

Thanks for your input, PaulOB.
I’m trying to re-factor my code so that I don’t have to hard code all of the repetitive logic. I’m not sure how to loop through the listelements for starters. This doesn’t work:

	for (const i = 1; i < 5; i++) {	
    let listelement[i] = document.createElement('li');
	liststart.appendChild(listelement[i]);
	}

Well it probably does, but you aren’t giving it a class, or an innerHTML, so it generates a generic list item element and appends it to your list, assuming that liststart and listelement are validly instantiated.

I wouldnt actually bother with creating an array, unless you need to refer to the elements in later pieces of code.
(I’m writing this at 10 PM. It probably needs revising and cleaning up, etc)

for(const i = 0; i < 5; i++) {
  let myli = document.createElement('li');
  myli.innerHTML = "TAB "+i;
  myli.addEventListener("click") function() { 
    Array.from(document.getElementsByClassName("active")).forEach((x) => { x.classList.remove("active"); }); //This line may be better written, but it's 10 PM.
    this.classList.add("active");
    document.getEelementById('section'+this.innerHTML.replace("TAB ","")).classlist.add("active")
    document.getEelementById('section'+this.innerHTML.replace("TAB ","")).scrollIntoView(true);
  }
  liststart.appendChild(myli);
}
1 Like

I commented out most of the code you provided because it wasn’t working and I left just this:

for(const i = 1; i < 5; i++) {
  let myli = document.createElement('li');
  myli.innerHTML = "TAB "+i;
  liststart.appendChild(myli);
}

This creates TAB 1 but nothing else. It doesn’t create the second through fourth tabs.

I switched from const i = 1 to let i = 1 and now it works.

2 Likes

oh right. Yeah, brain fart. The variable has to be able to be changed for the loop to work lol.

The addeventlistener has some syntax errors in it (told you it needed to be cleaned up)…

for(let i = 1; i < 5; i++) {
  let myli = document.createElement('li');
  myli.innerHTML = "TAB "+i;
  liststart.appendChild(myli);
  myli.addEventListener("click",function() { 
    [...document.getElementsByClassName("active")].forEach((x) => { x.classList.remove("active"); }); 
    this.classList.add("active");
    document.getElementById('section'+this.innerHTML.replace("TAB ","")).classList.add("active")
    document.getElementById('section'+this.innerHTML.replace("TAB ","")).scrollIntoView(true);
  });
}

(Also would probably recommend using some data attributes instead of trying to reference by innerHTML, but… topic for another thread, perhaps.)

1 Like

I had a go based on your previous code and came up with this :slight_smile:

const liststart = document.querySelector("#navbar__list");
for (var i = 1; i < 5; i++) {
  let myli = document.createElement("li");
  myli.innerHTML = "TAB " + i;
  myli.dataset.nav = "#section" + i;

  myli.addEventListener("click", function () {
    Array.from(document.getElementsByClassName("active")).forEach((x) => {
      x.classList.remove("active");
    }); //This line may be better written, but it's 10 PM.
    this.classList.add("active");
    document.querySelector(this.dataset.nav).classList.add("active");
    document.querySelector(this.dataset.nav).scrollIntoView(true);
  });
  liststart.appendChild(myli);
  document
    .querySelector("#navbar__list > li:first-child")
    .classList.add("active"); //set first item to active
  document.querySelector("#section1").scrollIntoView(true);
}

Probably could still be tidied up but I need the practice :slight_smile: (hope I didn’t break your code)

1 Like

Both of the code bases that you gave me work perfectly. I could have never come up with something so good. Thanks, PaulOB.

1 Like

I need for the section class name to switch to active when the windows scrolls to the section. Your code highlights the first section when TAB 1 is clicked like I want it to be but it doesn’t highlight the other sections when a TAB is clicked.

It was only highlighting the first section because I had written it in my stylesheet. Once I removed the css, it stopped highlighting the first section.

Never mind. I changed my stylesheet and now it works.

1 Like

Yes if you look at my codepen I changed the my-active class back to active and used it to highlight the tab and the section :slight_smile:

Felt left like a play as well :slight_smile:

Opted to split the code into a few different functions

window.addEventListener('DOMContentLoaded', () => {
  const doc = window.document

  const removeClassFromElements = (nodeList, classNames = '') => {
    nodeList.forEach(elem => elem.classList.remove(...classNames.split(' ')))
  }

  const navClickHandler = (event) => {
    removeClassFromElements(doc.querySelectorAll('.active'), 'active')

    const navItem = event.target
    const section = doc.querySelector(navItem.dataset.targetSection)

    navItem.classList.add('active')
    section.classList.add('active')
    section.scrollIntoView(true)
  }

  const createNavItem = (sectionNumber) => {
    const navItem = doc.createElement('li')

    navItem.textContent = `TAB ${sectionNumber}`
    navItem.dataset.targetSection = `#section${sectionNumber}`
    return navItem
  }

  const sections = doc.querySelectorAll('section[id^="section"]')
  const navList = doc.querySelector('#navbar__list')

  sections.forEach((section) => {
    const navItem = createNavItem(section.id.replace(/[^0-9]/g, ''))

    navList.appendChild(navItem)
    navItem.addEventListener('click', navClickHandler)
  })
})

commented

// Taking code out of the global context to avoid
// overwriting someone else's code and vice'n'versa
window.addEventListener('DOMContentLoaded', () => {
  const doc = window.document

  // helper function for removing classNames
  const removeClassFromElements = (nodeList, classNames = '') => {
    // Note: classList.remove can remove multiple classes at a time
    // e.g. classList.remove('active', 'open')
    // ...classNames.split(' ') converts: 'active open' -> ['active', 'open'] -> 'active', 'open'
    nodeList.forEach(elem => elem.classList.remove(...classNames.split(' ')))
  }

  // handler function called on mouse click
  // 'event' is the mouse click event object
  const navClickHandler = (event) => {
    // clear all elements with an active class
    removeClassFromElements(doc.querySelectorAll('.active'), 'active')

    // target property is the element we clicked on
    const navItem = event.target
    // access the dataset.targetSection attribute set earlier
    const section = doc.querySelector(navItem.dataset.targetSection)

    navItem.classList.add('active')
    section.classList.add('active')
    section.scrollIntoView(true)
  }

  // Making a separate function to create and return a new list element
  const createNavItem = (sectionNumber) => {
    const navItem = doc.createElement('li')

    navItem.textContent = `TAB ${sectionNumber}` // template string instead of concatenation
    navItem.dataset.targetSection = `#section${sectionNumber}` // nabbed from PaulOB :)
    return navItem
  }

  // get all sections with an id name that (^=) startswith "section"
  const sections = doc.querySelectorAll('section[id^="section"]')
  const navList = doc.querySelector('#navbar__list')

  // Brief: Adding more sections will automatically populate nav
  // looping through all the sections with forEach will handle that
  sections.forEach((section) => {
    const navItem = createNavItem(section.id.replace(/[^0-9]/g, ''))

    navList.appendChild(navItem)
    navItem.addEventListener('click', navClickHandler)
  })
})

codepen here

1 Like