Making text field taggable

Question

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>');
 aSpan.addClass('tag-label');
 var tagText= theText.val();
 aSpan.html(tagText);
 aTag.html(aSpan);
 allTags.prepend(aTag);
 theText.val('');
 }
 });
 }
})(jQuery);

Answer 1

Your code doesn't look too shabby! A few comments.

• 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;
 padding: 1em;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<ul></ul>

• toy: Thanks very much. But it looks like the wrap method doesn't work
• Schism: @toy Sorry about that. Try it out now! :-)