jQuery on click, add class but also remove if the class is already present-CodePudding

I have a nav menu that needs to trigger with clicks rather than hovers. When the links are clicked, an .open class would be added to the parent li. If that parent already has the .open class, then it would get removed. It would also be removed if another link is clicked on. So far I can get the class added when clicked and removed when a sibling is clicked, but not removed when it's already .open.

I tried adding a hasClass conditional, but that didn't work either. Seemed like it reruns the function every time it's clicked and therefore ignores the hasClass conditional.

Can anyone provide help? I tried toggleClass, but that didn't work.

$('li a').on('click', function() {
    $('li a').parent().removeClass('open');
    $(this).parent().addClass('open');
});

ul {
  list-style: none;
}

li {
  display: inline-block;
  padding: 10px;
}

.open {
  background-color: yellow;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<nav>
  <ul>
    <li>
      <a href="#">Item 1</a>
    </li>
    <li>
      <a href="#">Item 1</a>
    </li>
  </ul>
</nav>

CodePudding user response：

To do what you require you can use toggleClass() on the parent li when the element is clicked. To remove the class from all other li elements you can use removeClass() along with not() to exclude the current li. Try this:

$('li a').on('click', function() {
  let $li = $(this).parent().toggleClass('open');
  $('li').not($li).removeClass('open');
});

ul {
  list-style: none;
}

li {
  display: inline-block;
  padding: 10px;
}

.open {
  background-color: yellow;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<nav>
  <ul>
    <li>
      <a href="#">Item 1</a>
    </li>
    <li>
      <a href="#">Item 1</a>
    </li>
  </ul>
</nav>

CodePudding user response：

You can use

jquery toggleClass() to toggle yellow highlight (.open css class) on click/unclicking the same link.
jquery siblings() to remove .open class on all the other li items.

Below is the link for the demo

https://jsfiddle.net/so1u8hq6/

$('li a').on('click', function() {
        $(this).parent().siblings().removeClass('open');
    $(this).parent().toggleClass('open');
});

ul {
  list-style: none;
}

li {
  display: inline-block;
  padding: 10px;
}

.open {
  background-color: yellow;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

<nav>
  <ul>
    <li>
      <a href="#">Item 1</a>
    </li>
    <li>
      <a href="#">Item 2</a>
    </li>
    <li>
      <a href="#">Item 3</a>
    </li>
  </ul>
</nav>

CodePudding user response：

Late to the party, but, after seeing the provided answers and some of the CSS you use I had to urge with my suggestions:

UX. Avoid styling LI tags in general, or at least set the desired display and move on. Style directly the a tag (with the necessary paddings etc.). You'll not only get less CSS to take care of, but also a larger touch interaction area. Makes no sense to style something yellow if it's not a UI part of something interactable. Also in JS, you don't need to take care about the LI wrappers any more - but only about the actual A Elements.
Don't use common selectors like $('li a') - those might target any LI→A elements in your app. Instead be more specific and use a Class like i.e: .tabs for the parent UL. Both in CSS and JS.
Try to use Event Delegation (in jQuey using the .on() method). Not only it will help you to catch the Event.delegateTarget parent UL where needed, but also the this (the clicked element), but mainly reference all the "group" of a elements enclosed in the common parent. That way you can have as many .tabs in a single page as you like. And yes, thanks to Event delegation you can even add dynamically LI Elements - and your JS will still work as expected.
Since you're using <a href="#"> Anchor elements, instead of (more properly) <button type="button>" Elements, you need to also use Event.preventDefault() in order to prevent the browser its default behavior and that's to follow anchors (scroll the page, navigate, etc...)
Use the selector "a.open" when you want to target and remove the "open" class. By just using "a" (or in other answers on this page - "li") you're uselessly touching elements trying to remove a class that's not there in the first place.

Finally, here's the CSS retouch and the proper jQuery needed for your task:

$(".tabs").on("click", "a", function(ev) {
  ev.preventDefault();
  $("a.open", ev.delegateTarget).not(this).removeClass("open");
  $(this).toggleClass("open");
});

.tabs {
  display: flex;
  list-style: none;
  padding: 0;
}

/* Style your Anchors, not the dummy LI wrappers */
.tabs a      { padding: 10px; }
.tabs a.open { background-color: yellow; }

<ul >
  <li><a href="#">Item 1</a></li>
  <li><a href="#">Item 2</a></li>
  <li><a href="#">Item 3</a></li>
</ul>

<script src="//cdnjs.cloudflare.com/ajax/libs/jquery/3.6.0/jquery.min.js"></script>

To explain the only complicated line:

$(
  "a.open",             // Target just the ones (if any) of class "open"
  ev.delegateTarget     // inside the common ".tabs" ancestor
)
  .not(this)            // ... not the clicked element (since later we'll use .toggleClass on it)
  .removeClass("open"); // ... remove that class "open"

the rest is pretty self explanatory.

Further read:

CodePudding user response：

So you only want the yellow background to appear as a signifier of user interaction rather than for the background color to be displayed? Have you tried using the mousedown/mouseup functions instead of .on('click', function(){...}?

I was able to simulate the click event where the color showcases via this method:

$('li a').mousedown(function() {
  $('li a').parent().removeClass('open');
    $(this).parent().addClass('open');
});
$('li a').mouseup(function() {
  $('li a').parent().removeClass('open');
});

ul {
  list-style: none;
}

li {
  display: inline-block;
  padding: 10px;
}

.open {
  background-color: yellow;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<nav>
  <ul>
    <li>
      <a href="#">Item 1</a>
    </li>
    <li>
      <a href="#">Item 1</a>
    </li>
  </ul>
</nav>