I have two groups of two buttons (status). If you click on "Complete" or "On Hold", it will change the background color of the button. I am using same class name for all the buttons but each group of buttons have data attribute data-section='Park One'
and data-section='Park Two'
I am using the $.each()
function to detect the section, remove all the background and add the background-color on selected button. Is the code well coded? What can be improved?
jQuery:
$(".updateStatus").click(function () {
var buttonValue = $(this).val();
var section = $(this).data("section");
var cssClass = (buttonValue === "complete") ? 'green' : 'orange';
$('.updateStatus').each(function () {
if ($(this).data("section") == section) {
$(this).removeClass("green orange");
if ($(this).val() == buttonValue) {
$(this).addClass(cssClass);
}
}
})
});
HTML
<h2>Park One</h2>
<button class="updateStatus" value="complete" data-section='Park One'>Complete</button>
<button class="updateStatus" value="on hold" data-section='Park One'>On Hold</button>
<h2>Park Two</h2>
<button class="updateStatus" value="complete" data-section='Park Two'>Complete</button>
<button class="updateStatus" value="on hold" data-section='Park Two'>On Hold</button>
2 Answers 2
If you must use jQuery, then the simplest way by far is to wrap each section in a div :
<div class="section">
<h2>Park One</h2>
<button class="updateStatus" value="complete">Complete</button>
<button class="updateStatus" value="on hold">On Hold</button>
</div>
<div class="section">
<h2>Park Two</h2>
<button class="updateStatus" value="complete">Complete</button>
<button class="updateStatus" value="on hold">On Hold</button>
</div>
This allows you to use .siblings("button")
to select the clicked button's counterpart :
$(".updateStatus").click(function () {
$(this)
.addClass(($(this).val() === "complete") ? 'green' : 'orange')
.siblings("button").removeClass("green orange");
});
The .each()
loop vanishes.
It's very common to use divs or spans like this, to give your DOM structure.
I suggest a simplified version, based on the following reasoning:
- the first idea is to reduce the DOM lookup process; this can be achieved using a different method to target the involved buttons.
Instead of selecting all buttons of theupdateStatus
class and so having to loop with$('.updateStatus').each()
, we can select only the buttons of the clicked class:$('[data-section="' + this.dataset.section + '"]')
to remove the color classes on them - in the other hand, for affecting the button color, we can add a
data-color
class to each button: this way each one contains its own color, so the affectation becomes direct and independant of the context, as soon as the button has been clicked.
BTW there is no need to recognize the button by its value, so thevalue
attribute can be dropped.
The actual result is a reduced code, like this (see also this fiddle):
JS
$(".updateStatus").click(function () {
$('[data-section="' + this.dataset.section + '"]')
.removeClass('green orange');
$(this).addClass(this.dataset.color);
});
Note that for getting data-*
values we're using pure JS rather than jQuery: when jQuery doesn't bring an advantage, it's always a better choice from a performance point of view, because it uses a layer less.
HTML
<h2>Park One</h2>
<button class="updateStatus" data-section='Park One' data-color="green">Complete</button>
<button class="updateStatus" data-section='Park One' data-color="orange">On Hold</button>
<h2>Park Two</h2>
<button class="updateStatus" data-section='Park Two' data-color="green">Complete</button>
<button class="updateStatus" data-section='Park Two' data-color="orange">On Hold</button>
.btn
- not that they actually have to be buttons. \$\endgroup\$var $this = $(this)
andvar $updateStatus = $('.updateStatus')
\$\endgroup\$