Hours of operation input form

Question

I need some comments on the code that I "assembled". I want to know if it is efficient because I intend to use it on a live site.

jsFiddle

HTML:

<div id="hourForm">
<div id="Monday-Friday" class="day"></div>
<div id="Saturday" class="day"></div>
<div id="Sunday" class="day"></div>
</div>

jQuery:

$('.day').each(function() {
    var day = $(this).attr('id');
    $(this).append('<div id="label">' + day + ': </div>')
    .append('<select id="' + day + 'FromH" name="' + day + 'FromH" class="hour from"></select>')
    .append('<select id="' + day + 'FromM" name="' + day + 'FromM" class="min from"></select>')
    .append(' to <select id="' + day + 'ToH" name="' + day + 'ToH" class="hour to"></select>')
    .append('<select id="' + day + 'ToM" name="' + day + 'ToM" class="min to"></select>')
    .append(' <input type="checkbox" id="closed_' + day + '" name="closed" value="closed" class="closed" /> <label for="closed_' + day + '">Closed</label>');
});

$('.hour').each(function() {
    for (var h = 0; h < 24; h++) {
        $(this).append('<option value="' + h + '">' + h + '</option>');
    }

    $(this).filter('.from').val('6');
    $(this).filter('.to').val('22');
});

$('.min').each(function() {
    var min = [':00', ':15', ':30', ':45'];
    for (var m = 0; m < min.length; m++) {
        $(this).append('<option value="' + min[m] + '">' + min[m] + '</option>');
    }

    $(this).val(':00');
    $(this).filter('.from').val(':30');
});


$('input').change(function() {
    if ($(this).filter(':checked').val() == "closed") {
        $(this).siblings('select').attr('disabled', true);
    } else {
        $(this).siblings('select').attr('disabled', false);
    }
});

$('#Sunday .closed').val(["closed"]).siblings('select').attr('disabled', true);


function displayVals() {
    var MondayFridayFromHValues = $("#Monday-FridayFromH").val();
    var MondayFridayFromMValue = $("#Monday-FridayFromM").val();
    var MondayFridayToHValue = $("#Monday-FridayToH").val();
    var MondayFridayToMValue = $("#Monday-FridayToM").val();
    var MondayFridayClosedValue = $("#closed_Monday-Friday").filter(":checked").val();

    if (MondayFridayClosedValue == "closed") {
        var MondayFridayOpen = "Closed";
    }
    else {
        var MondayFridayOpen = MondayFridayFromHValues + MondayFridayFromMValue + "-" + MondayFridayToHValue + MondayFridayToMValue;
    }

    var SaturdayFromHValues = $("#SaturdayFromH").val();
    var SaturdayFromMValue = $("#SaturdayFromM").val();
    var SaturdayToHValue = $("#SaturdayToH").val();
    var SaturdayToMValue = $("#SaturdayToM").val();
    var SaturdayClosedValue = $('#closed_Saturday').filter(':checked').val();

    if (SaturdayClosedValue == "closed") {
        var SaturdayOpen = "Closed";
    }
    else {
        var SaturdayOpen = SaturdayFromHValues + SaturdayFromMValue + "-" + SaturdayToHValue + SaturdayToMValue;
    }

    var SundayFromHValues = $("#SundayFromH").val();
    var SundayFromMValue = $("#SundayFromM").val();
    var SundayToHValue = $("#SundayToH").val();
    var SundayToMValue = $("#SundayToM").val();
    var SundayClosedValue = $('#closed_Sunday').filter(':checked').val();

    if (SundayClosedValue == "closed") {
        var SundayOpen = "Closed";
    }
    else {
        var SundayOpen = SundayFromHValues + SundayFromMValue + "-" + SundayToHValue + SundayToMValue;
    }

    if (MondayFridayOpen == SaturdayOpen) {
        if (MondayFridayOpen == SundayOpen) {
            $("input:text[name='entry.7.group.other_option_']").val("Monday-Sunday: " + MondayFridayOpen);
        }
        else {
            $("input:text[name='entry.7.group.other_option_']").val("Monday-Saturday: " + MondayFridayOpen + ", Sunday: " + SundayOpen);
        }
    }
    else if (MondayFridayOpen != SaturdayOpen && SaturdayOpen == SundayOpen) {
        $("input:text[name='entry.7.group.other_option_']").val("Monday-Friday: " + MondayFridayOpen + ", Saturday-Sunday: " + SaturdayOpen);
    }
    else {
        $("input:text[name='entry.7.group.other_option_']").val("Monday-Friday: " + MondayFridayOpen + ", Saturday: " + SaturdayOpen + ", Sunday: " + SundayOpen);
    }
}
$("div#hourForm select").change(function() {
    $("input:radio[name='entry.7.group']:nth(3)").attr("checked", true);
});

$(":checkbox[name='closed']").change(function() {
    $("input:radio[name='entry.7.group']:nth(3)").attr("checked", true);
});

$("input:text[name='entry.7.group.other_option_']").click(function () {
    $("input:radio[name='entry.7.group']:nth(3)").attr("checked", true);
});

$("div#hourForm select").change(displayVals);
$(":checkbox[name='closed']").change(displayVals);
displayVals();

Hmm, at least the first part of your code ("building" select elements) shouldn't be done in JavaScript like that, but should be part of the static HTML, possibly generated using a server-side script. — RoToRa
– RoToRa, Commented Apr 2, 2012 at 10:19
@m1r0 Help doing what? Generate the HTML server-side? That would depend on the language and templating engine you use. And it would be off topic here. — RoToRa
– RoToRa, Commented Apr 5, 2012 at 14:49

Community · Accepted Answer · 2017-05-23 11:33:42Z

2

Some quick tips;

Look at caching $(this). You're regularly re-creating a jQuery object that doesn't change.
Adhere to the common JavaScript coding conventions; normally a variable that starts with a capital letter denotes a constructor (that should be invoked with new). Change all your SaturdayFromHValues etc variable names to saturdayFromHValues

As mentioned by @priteaes, you should save the HTML you're constructing to a string and write it to the element once. DOM manipulation is slow, and the browser will be re-paining the window with each append() you're using;

$(this).append('<div id="label">' + day + ': </div>')
    .append('<select id="' + day + 'FromH" name="' + day + 'FromH" class="hour from"></select>')
    .append('<select id="' + day + 'FromM" name="' + day + 'FromM" class="min from"></select>')

Becomes:

var html = '';
html += '<div id="label">' + day + ': </div>';
html += '<select id="' + day + 'FromH" name="' + day + 'FromH" class="hour from"></select>';
html += '<select id="' + day + 'FromM" name="' + day + 'FromM" class="min from"></select>';

$(this).append(html);

edited May 23, 2017 at 11:33

CommunityBot

1

answered Apr 2, 2012 at 15:18

Matt

1814 bronze badges

\$\begingroup\$ Great answer Matt, thank you! Do you mean something like this: jsfiddle.net/m1r0/Ace6h/27 . How can I add the other each to the append(html)? \$\endgroup\$

m1r0
– m1r0

2012-04-02 19:09:09 +00:00
Commented Apr 2, 2012 at 19:09
\$\begingroup\$ @m1ro: I wouldn't worry toooo much about getting each each into exactly 1 append(). What I'd change next would be the appends() in the for loops as well: jsfiddle.net/Ace6h/29 \$\endgroup\$

Matt
– Matt

2012-04-03 10:28:08 +00:00
Commented Apr 3, 2012 at 10:28
\$\begingroup\$ Here is the final form: jsfiddle.net/m1r0/Ace6h/31 . I learned a lot. Thanks again for helping me out. \$\endgroup\$

m1r0
– m1r0

2012-04-03 18:39:46 +00:00
Commented Apr 3, 2012 at 18:39

Add a comment |

Quill · Accepted Answer · 2015-08-30 14:26:09Z

In the first and second each, I would build the entire HTML first (as a string), and then append() once.

Update:

I prefer something like this:

$('.day').each(function() { 
    var day = $(this).attr('id'); 
    var html = '<div id="label">' + day + ': </div>' +
           '<select id="' + day + 'FromH" name="' + day + 'FromH" class="hour from"></select>' +
           '<select id="' + day + 'FromM" name="' + day + 'FromM" class="min from"></select>' + 
           ' to <select id="' + day + 'ToH" name="' + day + 'ToH" class="hour to"></select>' +
           '<select id="' + day + 'ToM" name="' + day + 'ToM" class="min to"></select>' +
           ' <input type="checkbox" id="closed_' + day + '" name="closed" value="closed" class="closed" /> <label for="closed_' + 
               day + '">Closed</label>';

    $(this).append(html);
});

Stack Exchange Network

Hours of operation input form

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Hours of operation input form

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions