4
\$\begingroup\$

I'm a UI / UX Designer trying to sharpen my JS skills. I wanted someone else to check this code snippet and maybe have a Pro tip to improve or shorten it.

Javascript:

var formContainer = $('.contact-form-container');
var formError = $('.form-error');
var tosLabel = $('.tos-label');
var tosCheckbox = $('.tos-checkbox');

function checkBoxValidation(tosLabel, tosCheckbox, formError) {
    if (!tosCheckbox.is(':checked')) {
        tosLabel.removeClass('tos-is-checked');
        formError.addClass('tos-error');
    } else if (tosCheckbox.is(':checked')) {
        tosLabel.addClass('tos-is-checked');
        formError.removeClass('tos-error');
    }
}

tosCheckbox.change(function() {
    checkBoxValidation(tosLabel, tosCheckbox, formError);
});

formContainer.submit(function () {
    checkBoxValidation(tosLabel, tosCheckbox, formError);
});

HTML:

    <form data-abide novalidate method="post" action="{{ path(route) }}" class="contact-form-container">
        {% if contact.error %}
            <div data-abide-error class="callout alert">
                <p class="alert-text"><i class="fi-alert size-24"></i>{{ contact.error }}</p>
            </div>
        {% elseif contact.success %}
            <div class="callout success">
                <p class="success-text"><i class="fi-check size-24"></i>{{ contact.success }}</p>
            </div>
        {% endif %}
        <div class="name-field">
            <label>Vor- und Nachname
                <input type="text" required pattern="[a-zA-Z]+" name="name" value="{{ contact.name }}">
                <small class="form-error">Vor- und Nachname angeben.</small>
            </label>
        </div>
        <div class="email-field">
            <label>Ihre E-Mail Adresse
                <input type="email" required name="email" value="{{ contact.email }}">
                <small class="form-error">Ihre E-Mail Adresse ist nicht korrekt.</small>
            </label>
        </div>
        <div class="message-field">
            <label>Ihre Nachricht
                <textarea placeholder="Schreiben Sie eine Nachricht" name="message" required>{{ contact.message }}</textarea>
                <small class="form-error">Ihre Nachricht angeben.</small>
            </label>
        </div>
        <div class="tos-field">
            <label for="checkbox1" class="tos-label">
                <input id="checkbox1" class="tos-checkbox" type="checkbox" name="privacy" {{ contact.privacy?'checked' }} required>
                Ich akzeptiere die Bestimmungen für den <a href="{{ path('tos') }}" class="tos-link">Datenschutz</a>.
            </label>
                <span class="form-error">Akzeptieren Sie die Bestimmungen zum Datenschutz.</span>
        </div>
        <input type="submit" class="button" value="Senden">
    </form>
\$\endgroup\$
  • \$\begingroup\$ So all '.tos-checkbox' in that form have to be checked to not give error? can you add also the forms HTML? \$\endgroup\$ – Sergio Jul 27 '16 at 6:07
  • \$\begingroup\$ Yes the .tos-checkbox (terms of service) checkbox must be checked to submit the form. \$\endgroup\$ – Lukas Hillebrand Jul 27 '16 at 6:18
  • 1
    \$\begingroup\$ $('.tos-checkbox') gives u ALL elements which have t. class '.tos-checkbox' upon it. That's a set. NOT a single element. In your case the set has just one element. But if you would extend your form so that you have two or more checkboxes with the class 'tos-checkbox' then it would become true if ANY of the checkbox becomes checked. Not deep into jQuery but obviously t. element which has become changed is t. this-keyword within t. handler-function. I guess jQuery passes that. Play around with this: codepen.io/mizech/pen/qNKAba?editors=1010 Perhaps u should use a more specific selector? \$\endgroup\$ – michael.zech Jul 27 '16 at 11:31
1
\$\begingroup\$

Here is a suggestion:

var tosCheckbox = $('.tos-checkbox');
var checkBoxValidation = (function(label, checkbox, errorDisplay) {
    var checked = checkbox.is(':checked');
    return function(e) {
        if (e.type == 'submit' && !checked) e.preventDefault();
        tosLabel.toggleClass('tos-is-checked', checked);
        formError.toggleClass('tos-error', !checked);
    }
})(tosLabel.closest('label'), tosCheckbox, formError.closest('label').find('.form-error'));

tosCheckbox.change(checkBoxValidation);
tosLabel.closest('form').submit(checkBoxValidation);

Baiscally I tried to make the code dependent/related to tosCheckbox, finding both label, error and form from that element. This avoids pointing to the wrong thing like '.form-error' which I see more than one in the page. This way only the one inside the label will receive the class.

DRYed also the event listener callbacks, the class add/remove and added a check to prevent the form submit event. Not sure you need this, but its a heads up in case you are not stoping the submit in case of error.

Hope it helps.

\$\endgroup\$

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.