4
\$\begingroup\$

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);
            })
        }
    })
\$\endgroup\$

1 Answer 1

1
+50
\$\begingroup\$

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 the data-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>

\$\endgroup\$
5
  • \$\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
    Commented Aug 23, 2022 at 23:43
  • \$\begingroup\$ Can I have a indeterminate status at this line ‘const checked = $(this).is(" :checked");’ ? \$\endgroup\$
    – Jefferson
    Commented 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\$ Commented 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
    Commented 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\$ Commented Aug 26, 2022 at 6:58

Not the answer you're looking for? Browse other questions tagged or ask your own question.