Change color of current selected element sidebar-CodePudding

I have a sidebar with site pages. I want to change the color of a currently selected page when clicked, assign the class is-active to the currently selected item, and remove it from all the others. Here is the HTML:

<aside id="side-bar" >
        <h3>Pages</h3>
        <nav >
            <a onclick="changeColor()"  >page 1</a>
            <a onclick="changeColor()"  >page 2</a>
            <a onclick="changeColor()"  >page 3</a>
        </nav>
</aside>

And CSS:

.sidebar .menu .menu-item:hover,
.sidebar .menu .menu-item.is-active {
    color: #f1672c;
    border-right: 6px solid #f1672c;
}

Each element has the function change color that should take all the elements, remove the is-active class from previous selected element and add to the curent clicked element.

function changeColor(){
    var links = document.getElementsByClassName("menu-item")
    links.map(classList.remove("is-active"))
    this.classList.add("is-active")
}

What am I missing?

CodePudding user response：

That changeColor() is not necessary....

const pages = document.querySelectorAll(".menu-item");

pages.forEach((item) => {
  item.addEventListener('click', active_item);
})

function active_item () {
  pages.forEach((item) => {
    item.classList.remove('is-active');
  });
  this.classList.add('is-active');
}

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

<aside id="side-bar" >
        <h3>Pages</h3>
        <nav >
            <a >page 1</a>
            <a >page 2</a>
            <a >page 3</a>
        </nav>
</aside>

CodePudding user response：

function changeColor(){
  var links = document.getElementsByClassName("menu-item")
  links.map(classList.remove("is-active"))
  this.classList.add("is-active")
}

A few things on that code:

getElementsByClassName doesn't return a regular array, so you cannot use map, forEach and other functions directly, use querySelectorAll instead
map and other array functions take a function as a parameter, the call should be something like links.forEach(link => link.classList...)
In your case this would be the window element, if you would like to pass the element from the onclick attribute then use changeColor(this)

Considering all that here is what you would have

function changeColor(el) {
  var links = document.querySelectorAll(".menu-item")
  links.forEach(link => link.classList.remove("is-active"))
  el.classList.add("is-active")
}

.sidebar .menu .menu-item:hover,
.sidebar .menu .menu-item.is-active {
  color: #f1672c;
  border-right: 6px solid #f1672c;
}

<aside id="side-bar" >
  <h3>Pages</h3>
  <nav >
    <a onclick="changeColor(this)" >page 1</a>
    <a onclick="changeColor(this)" >page 2</a>
    <a onclick="changeColor(this)" >page 3</a>
  </nav>
</aside>

CodePudding user response：

Just pass the reference of current element with this.

function changeColor(el){
    document.querySelector(".menu-item.is-active").classList.remove("is-active")
    el.classList.add("is-active")
}

.sidebar .menu .menu-item:hover,
.sidebar .menu .menu-item.is-active {
    color: #f1672c;
    border-right: 6px solid #f1672c;
}

<aside id="side-bar" >
        <h3>Pages</h3>
        <nav >
            <a onclick="changeColor(this)"  >page 1</a>
            <a onclick="changeColor(this)"  >page 2</a>
            <a onclick="changeColor(this)"  >page 3</a>
        </nav>
</aside>