How can I optimize this code that I wrote to make an accordion?-CodePudding

dear friends, I am just learning javascript and I wrote a code to make an accordion, but I feel that there are much better ways to implement this accordion, I would be grateful if you could guide me with the methods Get to know better. This is the code I wrote:

const inputs = document.querySelectorAll('input');
const contents = document.querySelectorAll('.content')

let activeTarget;
for (let i = 0; i < contents.length; i  ) {
    inputs[i].addEventListener("click",
        (event) => {
            if (contents[i].classList.value === "content open") {
                contents[i].classList.remove('open')
            }
            else if (activeTarget !== undefined) {
                activeTarget.classList.remove('open')
                contents[i].classList.add('open')
                activeTarget = contents[i]
            } else {
                contents[i].classList.add('open')
                activeTarget = contents[i];
            }
        }
        )
}

CodePudding user response：

Reduce the else if and else into just one else, then check if activeTarget is undefined in the new else. This reduces duplicated code.

Then let's change the condition in the if to use contains instead. This will be safer, because you don't check if the class list is exactly content open. Instead you only test if the element has the open class.

if (contents[i].classList.contains("open")) {
    contents[i].classList.remove('open')
} else {
    if (activeTarget !== undefined)
        activeTarget.classList.remove('open')
    contents[i].classList.add('open')
    activeTarget = contents[i]
}

CodePudding user response：

Rather than IF statements, you can just preemptively remove the 'open' class from all, then reapply it for the activeTarget

const inputs = document.querySelectorAll('input');
const contents = document.querySelectorAll('.content')
let activeTarget;

contents.forEach(item => item.addEventListener("click", e => {
  activeTarget = e.target;
  contents.forEach(i => i.classList.remove('open'));
  activeTarget.classList.add('open');
}))