I am working on a web page where there is a checkbox with a value and a hidden field. I want to ask your opinion on whether this logic is sound.
My logic
The checkbox has a value that matches the id of the hidden field. I have a function that sets all the hidden field values based on whether that checkbox is checked. If it's checked it will set all the hidden values to true if it's not checked it will set all the hidden fields to false.
html
<input type="checkbox" value="1" name="chkProduct">
<input value="true" id="sourceisSelected_1" type="hidden">
JS Script
$("[name='chkProduct']").click(function () {
var section = $(this).attr("value");
if ($(this).is(" :checked")) {
$("[id='sourceisSelected_" + section + "']").val(true);
$("[id='sourceSection_" + section + "']").show();
$("[data-value='" + section + "']").each(function (index, value) {
$(this).attr("disabled", false);
})
}
else {
$("[id='sourceisSelected_" + section + "']").val(false);
$("[id='sourceSection_" + section + "']").hide();
$("[data-value='" + section + "']").each(function (index, value) {
$(this).attr("disabled", true);
})
}
})
1 Answer 1
I want to ask your opinion on whether this logic is sound.
It is somewhat unclear how the values are used and thus difficult to judge the soundness of the logic. Perhaps in the server-side code there is a need to have a value of false
sent for an unchecked checkbox, which is not the default behavior.
One answer to Force a checkbox to always submit, even when unchecked suggests this approach of using a hidden input.
In some of the answers to Tri-state Check box in HTML? there is a suggestion of using the HTML5 Indeterminate state checkboxes though that likely doesn't help with getting the data on the server-side - perhaps it may only be helpful for hierarchical checkboxes, as the MDN documentation explains:
There are not many use cases for this property. The most common is when a checkbox is available that "owns" a number of sub-options (which are also checkboxes). If all of the sub-options are checked, the owning checkbox is also checked, and if they're all unchecked, the owning checkbox is unchecked. If any one or more of the sub-options have a different state than the others, the owning checkbox is in the indeterminate state.
Review
This code, while brief, is somewhat repetitive, which goes against the Don't Repeat Yourself principle. Between the if
block and the else
block the only differences appear to be true
, false
and the call to the .hide()
or .show()
method. This could be simplified by:
- storing the value used in the condition as a boolean variable
- utilizing that variable in the call to
.val()
, as well as calling the.toggle()
method and passing the negated value of the boolean value instead of the.hide()
and.show()
methods - removing the call to
.each()
and just calling the.attr()
on the jQuery collection of elements that match thedata-value
attribute, since that method will "set one or more attributes for every matched element."1
The code below illustrates such an approach. It also uses the const
keyword, which was mentioned in this previous review.
$("[name='chkProduct']").click(function() {
const section = $(this).attr("value");
const checked = $(this).is(" :checked");
$("[id='sourceisSelected_" + section + "']").val(checked);
$("[id='sourceSection_" + section + "']").toggle(checked);
$("[data-value='" + section + "']").attr("disabled", !checked);
});
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js" /></script>
<input type="checkbox" value="1" name="chkProduct" />
<input value="true" id="sourceisSelected_1" type="hidden">
<select data-value="1" disabled>
<option>Ferrari</option>
<option>Porche</option>
</select>
<div>
<input type="radio" data-value="1" disabled id="radio_1" name="prod1radio"><label for="radio_1">Radio 1</label>
</div>
<div>
<input type="radio" data-value="1" disabled id="radio_2" name="prod1radio"><label for="radio_2">Radio 2</label>
</div>
<div style="display: none" id="sourceSection_1"><img src="https://www.gravatar.com/avatar/02ab07612ab1bf8e8e49c909d21f9de0?s=64&d=identicon&r=PG" /></div>
-
\$\begingroup\$ Thanks for the information I am using the hidden field on the server side to see if the check box was checked selected data then unchecked. That is why I am setting all the hidden fields values. \$\endgroup\$Jefferson– Jefferson2022年08月23日 23:43:48 +00:00Commented Aug 23, 2022 at 23:43
-
\$\begingroup\$ Can I have a indeterminate status at this line ‘const checked = $(this).is(" :checked");’ ? \$\endgroup\$Jefferson– Jefferson2022年08月24日 01:36:46 +00:00Commented Aug 24, 2022 at 1:36
-
1\$\begingroup\$ I'm not sure what you mean by have an indeterminate status - it could be checked there - e.g.
this.indeterminate
though that would always be false given that it is in the callback to the click event. One could set it if desired:this.indeterminate = true;
\$\endgroup\$2022年08月24日 19:10:48 +00:00Commented Aug 24, 2022 at 19:10 -
\$\begingroup\$ I just dont understand how a checkbox can have indeterminate status. If its in the click event it should know . \$\endgroup\$Jefferson– Jefferson2022年08月24日 19:26:56 +00:00Commented Aug 24, 2022 at 19:26
-
1\$\begingroup\$ Maybe you already saw but in case not- I updated my answer to include text from MDN describing a use case for that property. Perhaps if that doesn’t match your use case then it won’t be very useful. \$\endgroup\$2022年08月26日 06:58:08 +00:00Commented Aug 26, 2022 at 6:58
Explore related questions
See similar questions with these tags.