1
\$\begingroup\$

In my application we have four inputs radio controls and on the load the value is checked to true. When the user select another radio and then save the form I want to get the new value. I am using the find class but then I need to use checked[0] is there a better way to do this?

JS

$("#form").submit(function (event) {
   event.preventDefault();
     console.log('free-text-form');
            var sortOrder = $(this).data('sortorder');
            console.log(sortOrder);
            var form = $(this);
            console.log('Processing Current Check');
            var checked = form.find(".radio-Class:checked");
            var currentChecked = checked[0];
            console.log(currentChecked);
            console.log(currentChecked.value);
            if (checkedRegRef.length == 1) {
                var NewId = currentChecked.value;
                 console.log(NewId);
            } 
});

Working Example https://jsfiddle.net/tjmcdevitt/vh9n5srL/87/

\$\endgroup\$
  • \$\begingroup\$ Error in console: Uncaught ReferenceError: checkedRegRef is not defined \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Apr 9 at 19:05
  • \$\begingroup\$ Sorry I corrected it \$\endgroup\$ – Jefferson Apr 10 at 12:48
2
+50
\$\begingroup\$

With checked[0] you are getting a reference to the actual DOM element (instead of the jQuery object) but that is unnecessary in your case since jQuery provides a val() method, which returns the current value of the first element in the set of matched elements. So in your case, you could simply do:

var checkedVal = form.find(".radio-Class:checked").val();

A better way to get the selected value as pointed out here by @Peter J is to use the input[name=radioName]:checked selector. Selecting through name attributes ensures that you select the desired radio group since these are meant to be unique. For better performance, you can pass in the form id as the second argument inside the selector method, which is used as a context here (this is same as if you would use $("#form").find("input[name=radioName]:checked")), here is the refactored code:

$("#form").submit(function(event) {
  var $formEl = $(this);
  var $labelEl = $formEl.find('#label1');
  var radioVal = $('input[name=RegimenReferences]:checked', $formEl).val();

  event.preventDefault();
  $labelEl.text(radioVal);
});
body {
  background: #20262E;
  padding: 20px;
  font-family: Helvetica;
}

#form {
  background: #fff;
  border-radius: 4px;
  padding: 20px;
  font-size: 25px;
  text-align: center;
  transition: all 0.2s;
  margin: 0 auto;
  width: 300px;
}

button {
  background: #0084ff;
  border: none;
  border-radius: 5px;
  padding: 8px 14px;
  font-size: 15px;
  color: #fff;
}

#banner-message.alt {
  background: #0084ff;
  color: #fff;
  margin-top: 40px;
  width: 200px;
}

#banner-message.alt button {
  background: #fff;
  color: #000;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<form id="form" class="form-horizontal" data-sortorder="1">
  <p>Update Values</p>
  <label id="label1" type="text">1</label>
  <div>
    <label for="radio1">
      <input type="radio" id="radio1" name="RegimenReferences" value="1" class="radio-Class" checked='true'>
      (a) aaaaaa
    </label>
  </div>
  <div>
    <label for="radio2">
      <input type="radio" id="radio2" name="RegimenReferences" value="2" class="radio-Class">
      (b) bbbbbb
    </label>
  </div>
  <div>
    <label for="radio3">
      <input type="radio" id="radio3" name="RegimenReferences" value="3" class="radio-Class">
      (c) cccccc
    </label>
  </div>
  <div>
    <label for="radio4">
      <input type="radio" id="radio4" name="RegimenReferences" value="4" class="radio-Class">
      (c) cccccc
    </label>
  </div>
  <div class="text-center">
    <button type="submit" class="btn btn-primary modal-submit-btn">Save</button>
    <button type="button" class="btn btn-default modal-close-btn" data-dismiss="modal">Close</button>
  </div>
</form>

Edit 1: Since you already got the form selected you can pass that as the context therefore you don't need the form id.

| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

Your question

I am using the find class but then I need to use checked[0] is there a better way to do this?

As was already answered, the val() method can be used to get the value of the first element matched in the collection. Additionally .eq() could be used to get a reference to the first element if necessary.

The accepted answer to In jQuery, how do I get the value of a radio button when they all have the same name? mentions both the jQuery .val() method, as well as a vanilla Javascript technique.

Because this answer keeps getting a lot of attention, I'll also include a vanilla JavaScript snippet.

It is wise to consider whether you really need jQuery on your page. Take a look at youmightnotneedjquery.com/ (and also this article). If you decide to eliminate it, you could just access the form elements via HtmlFormElement.elements and the forms property.

const label = document.getElementById('label1');
document.forms[0].addEventListener('submit', event => {
  event.preventDefault();
  label.innerHTML = document.forms[0].elements.RegimenReferences.value;
});

With the approach above there are no function calls to query the DOM for the elements in the form submission callback handler. This may not be much faster but would require fewer function calls.

Other review points

  • Indentation is inconsistent - the first line in the callback function is indented with three spaces, then the next line is indented with a tab and a space and then subsequent lines appear to be indented with twelve spaces. It is best to use consistent indentation for the sake of readability.
  • DOM lookups aren't cheap so it is wise to store those in variables - e.g. const label as defined in the snippet above.
  • There is an Unused Variable: sortOrder (other than being logged to the console)
  • It is wise to use strict equality comparisons unless there is a chance one operand might not have the same type. For example - instead of:

    if (checkedRegRef.length == 1) {
    

    use ===

    if (checkedRegRef.length === 1) {
    
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Just out of curiosity and this might not be the best example, but when you say "DOM lookups aren't cheap so it is wise to store those in variables", isn't this debatable in this case if the label is intended to only change after the form is submitted? I always assumed that you don't want to store the DOM lookup by default and only if it's worth it. So my thinking here was: "I have a label which only changes after the user clicks the button, I don't want to bloat the memory until then". Even this is a very insignificant case, do I raise a valid point ? \$\endgroup\$ – htmn Apr 15 at 15:46
  • 1
    \$\begingroup\$ Yes it is debatable, and depends on various factors like how often the user would likely submit the form. In the "best case" scenario the user might only submit the form once or twice, leading to the callback function getting called and consequently the call to .find() to lookup the label element. In the "worst case" scenario the form might get submitted multiple times, leading to that lookup each time the callback is executed. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Apr 15 at 15:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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