I've worked hard on this code and it functions exactly as I wanted, but I've been told it is a little bit unreadable. My focus is to get the code as clean as possible, and have things as efficient as possible.
This is the code I am working with: http://jsfiddle.net/xT5X5/6/
//latest
var maxFields = 10;
$('.form').on('click', '.add', function () {
var value_src = $(this).prev();
var container = $(this).parent().prev();
if ($.trim(value_src.val()) !== '') {
if (container.children().length < maxFields) {
var value = value_src.val();
var html = '<div class="line">' +
'<input class="accepted" type="text" value="' + value + '" />' +
'<input type="button" value="X" class="remove" />' +
'</div>';
$(html).appendTo(container);
value_src.val('');
} else {
alert("You tried to add a field when there are already " + maxFields);
}
} else {
alert("You didn't enter anything");
}
})
.on('click', '.remove', function () {
$(this).parents('.line').remove();
});
$(".current").keyup(function(e) {
if (e.which == 13) {
$(this).next("input").trigger("click");
}
});
$(document).on("keyup",".accepted",function(e) {
if (e.which == 13) {
$(this).closest('.copies').next().find(".current").focus();
}
});
Can anyone give me any pointers on how I've done, and if there are any improvements that you can spot. Any feedback would be much appreciated!
Thank you.
1 Answer 1
First of all there is a great article on refactoring jQuery by Jack Franklin.
Second is my solution that i came up with in last 2 hours(ocd kicked in). My solution is probably not right, because i still have a ton to learn and would appreciate any feedback !!
Here fiddle with implementation. Here is a short version of what is going on inside, as you can see i tried to refactor each action in it's own method:
// detail object that handles our form
var details = {
maxFields: 10,
form: "",
init: function(el) {},
bindEvents: function() {},
appendCopy: function(event) {},
removeCopy: function(event) {},
createCopy: function(value) {},
focusOnEnter: function(event) {},
addCopyOnEnter: function(event) {},
_isValueEmpty: function(val) {},
_isMaxReached: function(copyContainer) {},
_getParent: function(event){},
_getCurrent: function(event){}
_getCopyContainer: function(event){},
_getValue: function(event){},
};
// initialize our details object
details.init(".form");
I think this could also be rewritten using $.deferred
where each deferred
would be responsible for tracking it's own progress (requirements,benefits,qualifications).
.map()
will suffice. \$\endgroup\$