HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Changing the background color when a link is clicked

Submitted by: @import:stackexchange-codereview··
0
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 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 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.