CODEWITHSUNDEEP
javascriptjquerycssmenu
Hardist
Hardist

Reputation: 1993

Javascript onclick menu change background colors

I have my menu like this:

https://jsfiddle.net/23r4q610/

And my code to change the selected menu button like below:

$('#bluebutton').click(function () {
    $('.testul li').removeClass('selectedred selectedpurple selectedgreen selectedorange');
    $('#bluebutton').addClass('selectedblue');
});

$('#redbutton').click(function () {
    $('.testul li').removeClass('selectedblue selectedpurple selectedgreen selectedorange');
    $('#redbutton').addClass('selectedred');
});

$('#purplebutton').click(function () {
    $('.testul li').removeClass('selectedblue selectedred selectedgreen selectedorange');
    $('#purplebutton').addClass('selectedpurple');
});

$('#greenbutton').click(function () {
    $('.testul li').removeClass('selectedblue selectedred selectedpurple selectedorange');
    $('#greenbutton').addClass('selectedgreen');
});

$('#orangebutton').click(function () {
    $('.testul li').removeClass('selectedblue selectedred selectedpurple selectedgreen ');
    $('#orangebutton').addClass('selectedorange');
});

Ofcourse this is bad code since it could be written much shorter. Should I go about this using just numbers so I can do some foreach, or is there a better way to do this?

Upvotes: 0

Views: 2231

Answers (4)

depperm
depperm

Reputation: 10746

This can be condensed by adding a generic click event on all buttons by using [id*="button"]. Then grab the relevant color from the nested anchor.

$('[id*="button"]').click(function(){
    $('.testul li').removeClass();
    $(this).addClass('selected'+$('a',this).attr('class'));
});

or

$('li').click.../*this would be the same as above*/

fiddle

Upvotes: 4

Nikhil Aggarwal
Nikhil Aggarwal

Reputation: 28455

You can reduce the code to only 1 click binding. Where when an element is clicked, class from all the li's is removed and then on the current clicked li, selected class is added.

$(".testul > li").click(function(){
$('.testul li').removeClass('selectedred selectedpurple selectedgreen selectedorange selectedblue');
    var color = $(this).attr("id").replace("button","");
    $('#'+color+'button').addClass('selected'+color);

});

Here is the updated fiddle - https://jsfiddle.net/23r4q610/2/

Upvotes: 0

Gábor Nagy
Gábor Nagy

Reputation: 189

I would avoid hard-coding the color names into the HTML IDs. Rather use a CSS class name like "selected" and describe in your CSS what that should look like. Example:

<li id="home-button" class="color-button"><a href="#">Home</a>

CSS:

#home-button.selected,
#home-button:hover {
    background: -webkit-linear-gradient(#78b1ff, #4881dc);
}

JS:

$('.color-button').click(function () {
    $(this).toggleClass("selected").siblings(".color-button").removeClass("selected");
}

This way color information (presentation) is separated from semantic information (like "home") and JS code is daramtically shorter.

Note: this is just an advice, I have not tested it but should give you a good point to start.

Upvotes: 0

Organiccat
Organiccat

Reputation: 5651

In this particular case, there doesn't appear to be a good reason to add and remove classes. Just change the background color instead of adding and removing a class to do so.

$(this).css("background-color", "red");

Upvotes: 0

Related Questions