patternjavascriptMinor
Changing the background color when a link is clicked
Viewed 0 times
thebackgroundcolorclickedwhenchanginglink
Problem
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
jsFiddle
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");
});
});Solution
The opening and closing tags don't match in your JSFiddle.
It's not a good idea to use presentational like
It would be a lot simpler if you removed the
In addition, I suggest using
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;
}
- Clients
- Services
- Add
- Time
In addition, I suggest using
text-transform: uppercase to make the text all caps, as that is also a presentation style choice.Context
StackExchange Code Review Q#80377, answer score: 3
Revisions (0)
No revisions yet.