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");
 });
});
asked Feb 12, 2015 at 19:45
\$\endgroup\$
0

3 Answers 3

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.

answered Feb 12, 2015 at 20:19
\$\endgroup\$
1
  • \$\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\$ Commented Feb 13, 2015 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 }
answered Feb 14, 2015 at 4:38
\$\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");
 });
});
answered Feb 12, 2015 at 20:19
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.