4
\$\begingroup\$

What I'm trying to do is to change the background color when a link is clicked (in this case a list item). When it is no longer clicked, it should revert back to its normal color.

I was able to get this to work via the jQuery addClass and removeClass methods, but this seems to be hardcoding it.

jsFiddle

$(document).ready(function() {
    $(".nav > li:first-child").click(function(){
      $(this).addClass("highlighted");
      $(".nav > li:nth-child(2), .nav > li:nth-child(3), .nav > li:last-child").removeClass("highlighted");
    });
    $(".nav > li:nth-child(2)").click(function(){
        $(this).addClass("highlighted");
        $(".nav > li:first-child, .nav > li:nth-child(3), .nav > li:last-child").removeClass("highlighted");
    });
    $(".nav > li:nth-child(3)").click(function(){
        $(this).addClass("highlighted");
        $(".nav > li:first-child, .nav > li:nth-child(2), .nav > li:last-child").removeClass("highlighted");
    });
    $(".nav > li:last-child").click(function(){
        $(this).addClass("highlighted");
        $(".nav > li:first-child, .nav > li:nth-child(2), .nav > li:nth-child(3)").removeClass("highlighted");
    });
});
\$\endgroup\$
3
\$\begingroup\$

The opening and closing tags don't match in your JSFiddle.

It's not a good idea to use presentational like highlighted for a CSS class name. It would be better to use current or selected, and let the stylesheet decide how to indicate which item is currently selected — whether by highlighting, underlining, or whatever visual hint it chooses.

It would be a lot simpler if you removed the highlighted class from all items, then added it back for just the selected item.

$(document).ready(function() {
    $(".nav > li").click(function() {
        $(".nav > li").removeClass('current');
        $(this).addClass('current');
    });
});
* {
	margin: 0px;
	padding: 0px;
}

nav {
	background-color: #e4801c;
	height: 100%;
}
.nav > li {
	text-align: center;
	text-transform: uppercase;
}

.current {
	background: #faaf5e;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.10.1/jquery.min.js"></script>

<nav class="navbar navbar-default" role="navigation">
        <ul class="nav navbar-nav">                      
          <li>Clients</li>
          <li>Services</li>
          <li>Add</li>
          <li>Time</li>
        </ul>
</nav>

In addition, I suggest using text-transform: uppercase to make the text all caps, as that is also a presentation style choice.

\$\endgroup\$
  • \$\begingroup\$ Also, IMHO the selector $(".nav > li") should be saved in a variable and reused whithin the click handler, and the $(this).addClass call may be replaced with this.classList.add (better perf) if the OP don't need to support IE9 and below (caniuse.com/#search=classlist) \$\endgroup\$ – pomeh Feb 13 '15 at 8:36
2
\$\begingroup\$

I know this isn't doing it "in javascript", but you seriously should take advantage of all native code that you possibly can --

Just do this in CSS.

Make them all links, and add a class, such as

a.mylink:active { font-style: bold }
\$\endgroup\$
1
\$\begingroup\$

Instead of having one handler for each element, you can have one for all, that first removes the class on any of the elements that has it, than adds it on the right element:

$(document).ready(function() {
    $(".nav > li").click(function(){
      $(".nav > li.highlighted").removeClass("highlighted");
      $(this).addClass("highlighted");
    });
});
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.