I have the following code:
- The HTML part consist of two containers. Both containers have a list of hour and day weeks.
- This code is used to manage these lists of inputs.
The JavaScript code works but I am wondering if it is possible to improve it, because the JavaScript code consists of around about 230 lines of codes.
This is maybe too much considering I am using jQuery and Underscore library. Any hints to improve/clean/slim the code?
/*global jQuery, window */
(function (,ドル _, window) {
'use strict';
var $body = $('body');
var translator = window.ExposeTranslation;
var copyValues,
hasSameValues,
checkInputs,
createToggleButton;
var selector = {
updateContest: $('.update-contest'),
reminderContest: $('.reminder-contest'),
periodicityDaysClassName: '.data-periodicity-days',
periodicityHoursClassName: '.data-periodicity-hours',
btnToggleReminderClassName: '.btn-toggle-reminders'
};
var DAYS_MAP = {
daily: {0: true, 1: true, 2: true, 3: true, 4: true, 5: true, 6: true}, // daily
weekday: {0: true, 1: true, 2: true, 3: true, 4: true, 5: false, 6: false}, // weekdays
every_mon_wed_fri: {0: true, 1: false, 2: true, 3: false, 4: true, 5: false, 6: false}, // every_mon_wed_fri
every_tue_thu: {0: false, 1: true, 2: false, 3: true, 4: false, 5: false, 6: false} // every_tue_thu
};
var HOURS_RANGE = _.extend({}, _.map(_.range(24), function (i) {
return (i === 18);
}));
var createSelectElement = function (items, parentElement, callback) {
var selectElement = $('<select>'),
optionElement;
$.each(items, function (key, value) {
optionElement = $('<option>').val(key).text(value);
selectElement.append(optionElement);
});
parentElement.prepend(selectElement);
if (callback) {
selectElement
.on('change', callback)
.on('change', function () {
// it sets the same value in the select of Reminder section
if ($(selector.btnToggleReminderClassName).hasClass('active')) {
$(selector.periodicityDaysClassName + ' select').val($(this).val());
copyValues();
}
});
}
};
var selectDay = function () {
var $this = $(this),
labels = $(selector.periodicityDaysClassName).find('label'),
inputs = labels.find('input'),
value = $this.val(),
setProp;
setProp = function (dayRange) {
$.each(dayRange, function (i, val) {
$(inputs[i]).prop("checked", val);
});
labels.hide();
};
if (value === 'daily') {
labels.hide();
inputs.prop('checked', true);
} else if (value === 'weekday') {
setProp(DAYS_MAP[value]);
} else if (value === 'weekly') {
labels.show();
labels.find('input').prop('checked', false);
$(inputs[(new Date()).getDay()]).prop('checked', true);
} else if (value === 'every_mon_wed_fri') {
setProp(DAYS_MAP[value]);
} else if (value === 'every_tue_thu') {
setProp(DAYS_MAP[value]);
}
};
var selectHours = function () {
var value = $(this).val(),
labels = $(selector.periodicityHoursClassName).find('label'),
inputs = labels.find('input');
if (value === 'customize') {
labels.show();
} else {
$(inputs).prop('checked', false);
$(inputs[value]).prop('checked', true);
labels.hide();
}
};
checkInputs = function () {
// it set the select on the right place on load document
var setSelectDayValue = function (contest) {
var periodicityDaysInputs = contest.find(selector.periodicityDaysClassName + ' input'),
currentDaysRange = {};
$.each(periodicityDaysInputs, function (i, v) {
currentDaysRange[i] = $(v).prop('checked');
});
// it sets the default value of days select to weekly
contest.find(selector.periodicityDaysClassName + ' select').val('weekly');
$.each(DAYS_MAP, function (index, dayRange) {
if (_.isEqual(dayRange, currentDaysRange)) {
contest.find(selector.periodicityDaysClassName + ' select').val(index);
contest.find(selector.periodicityDaysClassName + ' label').hide();
return 0;
}
});
};
var setSelectHourValue = function (contest) {
var periodicityHoursInputs = contest.find(selector.periodicityHoursClassName + ' input'),
currentHoursRange = {};
$.each(periodicityHoursInputs, function (i, v) {
currentHoursRange[i] = $(v).prop('checked');
});
// it sets the default value of 'hour select element' to customize
contest.find(selector.periodicityHoursClassName + ' select').val('customize');
if (_.isEqual(currentHoursRange, HOURS_RANGE)) {
contest.find(selector.periodicityHoursClassName + ' select').val(18);
contest.find(selector.periodicityHoursClassName + ' label').hide();
}
};
setSelectDayValue(selector.updateContest);
setSelectDayValue(selector.reminderContest);
setSelectHourValue(selector.updateContest);
setSelectHourValue(selector.reminderContest);
if (hasSameValues()) {
$(selector.btnToggleReminderClassName).click();
}
};
createToggleButton = function (parentElement) {
var buttonElement = $('<button>'),
isVisible;
buttonElement.text('copy update periodicity');
buttonElement.attr({
'class': 'btn ' + selector.btnToggleReminderClassName.substring(1),
'type': 'button',
'data-toggle': 'button',
'data-complete-text': 'customize'
});
parentElement.prepend(buttonElement);
buttonElement.on('click', function (event) {
// TODO it works but it is triggered two times, I should have a container of these two elements
$(event.currentTarget).parent('.reminder-contest').find('.control-group').toggle(function () {
isVisible = $(this).is(':visible');
buttonElement.button(isVisible ? 'reset' : 'complete');
if (!isVisible) {
selector.reminderContest.find('.form-help-block').show();
} else {
selector.reminderContest.find('.form-help-block').hide();
}
});
});
};
copyValues = function () {
var container1 = selector.updateContest.find('input:checkbox');
var container2 = selector.reminderContest.find('input:checkbox');
var updateDaysSelect = selector.updateContest.find('select');
var reminderDaysSelect = selector.reminderContest.find('select');
container2.each(function (i) {
$(this).prop('checked', $(container1[i]).prop('checked'));
});
reminderDaysSelect.val(updateDaysSelect.val());
};
hasSameValues = function () {
var result = true,
updateInputs = selector.updateContest.find('input:checkbox'),
reminderInputs = selector.reminderContest.find('input:checkbox');
reminderInputs.each(function (i) {
if ($(this).prop('checked') !== $(updateInputs[i]).prop('checked')) {
result = false;
return false;
}
});
return result;
};
$(function () {
var ITEM_DAYS = {
daily: 'daily',
weekday: 'weekday',
weekly: 'weekly',
every_mon_wed_fri: 'every_mon_wed_fri',
every_tue_thu: 'every_tue_thu'
};
var ITEMS_HOURS = {
18: 'at 18 pm',
customize: 'customize'
};
// it creates select for Days and Hours labels
$.each($(selector.periodicityHoursClassName), function (i, element) {
createSelectElement(ITEMS_HOURS, $(element), selectHours);
});
$.each($(selector.periodicityDaysClassName), function (i, element) {
createSelectElement(ITEM_DAYS, $(element), selectDay);
});
// it creates the button for copy the "Update Periodicity" into "Reminder Periodicity"
createToggleButton(selector.reminderContest);
// it checks the initial values in order to set properly the select and button elements values
checkInputs();
$body.on('click', selector.updateContest.find('input:checkbox'), function () {
var copyUpdatePeriodicityBtn = $(selector.btnToggleReminderClassName);
if (copyUpdatePeriodicityBtn.hasClass('active')) {
copyValues();
}
});
selector.updateContest.closest('.control-group').addClass('periodicity-fields-container');
selector.reminderContest.closest('.control-group').addClass('periodicity-fields-container');
selector.reminderContest.prepend('<div class="form-help-block">'+'messages:contest.help.customize_periodicity' + '</div>');
});
}(jQuery, window._, window));
1 Answer 1
Considering the complexity of your GUI I'd say your code is already quite trim. If you wish to cut it down I'd go about simplifying the user interface, personally I found it confusing - but then again I am using it out of context.
Whilst it's great from a code view point to reuse interfaces (i.e the duplication between update and reminder sections) - from a user POV this can be confusing when the repetition appears constantly.
If it were me I'd take your generalised code for handling your date, repeat and time fields and abstract it away behind your own form widget, something like:
+------------------+
Summary text about repeat date and time | click to change |
+------------------+
With this kind of widget you could rework your interface:
Updates:
+------------------+
Repeats weekly, every Tuesday, at 18:00 and 20:00 | click to change |
+------------------+
Reminders:
+--------------+
/no reminders set/ | click to set |
+--------------+
[ ] tick to synchronize Reminders with Updates
Once you have that kind of layout, you'd just need to rework your code so that using either widget calls into existence your generalised GUI for setting a date, frequency and time - you could use a popover, or a new admin screen. By tidying this code away in a reusable widget you'd probably find places where you could abstract and cut down code (i.e. not having to be specific with class names or input synchronisation).
Also, if you developed a interpretable and generalised way of describing a repeat event, like:
{repeats:'weekly',every:'tuesday','at':[18:00,20:00]}
This could be stored in a hidden field or attribute along with the widget once used. With this, syncronising the information between widgets suddenly becomes quite easy, wouldn't require specific fields to be constantly harmonised, and any harmonising work could be done in a simple function outside of all the widget code.
Doing things in the manner above would also help with extensibility, if you ever needed to add an extra type of event — or wanted to allow the user to define multiple update events. You would be able to add a new widget easily.
Anyway, apologies if this wasn't the answer you were looking for, as I said the code to me looks pretty good for what you have — it's more the approach that could improve things.
-
\$\begingroup\$ +1 thanks very much for having a look at my code. Your comments are greatly appreciated. \$\endgroup\$Lorraine Bernard– Lorraine Bernard2012年10月14日 07:42:56 +00:00Commented Oct 14, 2012 at 7:42
Explore related questions
See similar questions with these tags.