# Making text field taggable

I'm new to JavaScript and new to jQuery plugin development. My plugin is really easy I just want to taggify a <ul>

    (function($) { var allTags;$.fn.taggify = function(options) {
allTags = this;
_create(allTags);
return this;
};

function _create(theElement) {
var inputWrapper = $('<li></li>'); var input =$('<input></input>');
input.attr('type', 'text');
input.attr('autocomplete', 'off');
input.css('border', 'none');
input.css('outline', 'none');
inputWrapper.html(input)

allTags.append(inputWrapper);
_event(input);
}

function _event(theText) {
theText.keyup(function(e) {
if(e.keyCode === 13) {
var aTag = $('<li></li>'); aTag.addClass('tag-choice'); var aSpan =$('<span></span>');

var tagText= theText.val();
aSpan.html(tagText);
aTag.html(aSpan);

allTags.prepend(aTag);
theText.val('');
}
});
}

})(jQuery);


• Declaring but then not using options made me stumble. If you don't have any options, don't make a parameter called options.

• In my opinion there's not much point in separating _create and _event into two separate functions. If you do, you should rename _event so that it's more like a function name. Functions should be named such that they start with a verb; try something like _attachListener.

• It's fine to use a closure-scope variable, but you're kind of half-using it. If you combine your two functions, you'll be able to get rid of it entirely.

• _create isn't using its theElement parameter.

• You should take advantage of method chaining when creating input. Also, consider naming jQuery variables with a leading $ to make it very explicit what type they are. • The .html() function does not take a jQuery object as a valid parameter; it takes either an htmlString or a function. Instead you should use inputWrapper.append(input);. • In fact, rather than creating an inputWrapper at all, use the .wrap() function. • Do you really want to use aSpan.html(tagText);, or do you want to use .text()? This depends on your use case but usually people will expect the behaviour of .text() (e.g. not having to escape their angle brackets). • For consistency, either name them both anInput and aSpan or drop the prefix. • Note that using function chaining, you can in fact completely eliminate the need for having any variables at all, if you prefer. Here's my proposed rewrite: (function($) {
$.fn.taggify = function(options) { create(this); return this; }; function create($theElement) {
var $input =$('<input></input>')
.attr('type', 'text')
.attr('autocomplete', 'off')
.css('border', 'none')
.css('outline', 'none')
.wrap('<li></li>');

$input.on('keyup', function(e) { if (e.keyCode === 13) { var tagText =$input.val();
var $span =$('<span class="tag-label"></span>');

$span.text(tagText).wrap('<li class="tag-choice"></li>');$theElement.prepend($span.parent());$input.val('');
}
});

$theElement.append($input.parent());
}
})(jQuery);

$(function() {$('ul').taggify();
});
ul {
background: #ccc;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<ul></ul>