Custom Kit Form Validation

Question

I have an HTML form where I need to do the following

• when value of Select Box changes to initial "Not Selected" value (where values of the option is 0), I am to highlight the select box, when it's non-0, restore the original coloring of the select
• when I submit the form, and the Select box value is 0, do not submit the form, and highlight the box

Doing all of this left me with code duplication. See highlight and restore

How do I get rid of the duplication? Can I?

highlight("kit_model");
$('#kit_submit').prop("disabled", true);

$("#kit_model").change(function() {

    if ($("#kit_model").val() == 0)
    {
        highlight("kit_model");
        $('#kit_submit').prop("disabled", true);
    }
    else
    {
        restore("kit_model");
        $('#kit_submit').prop("disabled", false);
    }

});
$("#custom_kit_form").submit(function() {
    if ($("#kit_model").val() == 0)
    {
        highlight("kit_model");
        return false;
    }

    $.post(
            "web.php
            $("#custom_kit_form").serialize(),
            $("#main").load('main.php')
         );

    return false;
});

Form

<form action="" id="custom_kit_form" method="post" name="custom_kit_form">
    <table>
        <tr>
            <td><label for="kit_model" id="label_kit_model">FS Kit</label></td>

            <td><select id="kit_model" name="kit_model">
                    <option value="0">-- SELECT --</option>

                    <option value="573">TUBING</option>
            </select></td>
        </tr>

        <tr>
            <td></td>

            <td><input id="kit_submit" name="kit_submit" type="submit" value="Save"></td>
        </tr>
    </table>
</form>

Answer 1

Way too many calls to $

This pops up way too much in your code (or something like this):

$("#kit_model")

Normally, document.getElementById itself can be slow. However, this jQuery function is even slower because, not only does it have to traverse the DOM in search for a single element, but it also has to wrap it sugar another object for access to more methods/properties.

It'd be better to reduce your calls to this function. Instead, store the return from a single call in a variable and use that:

var kitModel = $("#kit_model");

Do this for the other elements that you grab more than once, too.

If you are in control of the highlight and restore functions, change them so they can accept the element itself through parameters. This will make your function more flexible, and also allows you to just continue to use this variable you have.

---

Tables for layout

If I understand correctly, you are using tables in your HTML code to correctly position your DOM elements. This is bad/old practice. You should instead be using CSS rules to position elements.

Tables are for data that belongs in tables.

---

Initialize attributes in HTML

Here, you are initializing your submit button to be disabled in your JavaScript code:

$('#kit_submit').prop("disabled", true);

Instead, you should simply just slap a disabled onto the element itself.

---

Unnecessary condition

Will this condition ever pass?

$("#custom_kit_form").submit(function() {
    if ($("#kit_model").val() == 0)

The submit button can't be clicked in the first place if the value of the kit model is 0 because the change listener will disable the button.

You can safely remove this part of the submit listener.