Bootstrap file input styler

Question

This simple JavaScript code styles all file inputs to match with Bootstrap's style.

Simply, it turns this:

Unstyled input

Into this input:

Styled input

---

(function(window){
 window.jQuery(function($){
 $('input[type="file"]').each(function(){
 var $this = $(this);
 var $parent = $this.parent();
 $this.detach();
 $parent.prepend(
 $('<div id="' + $this.attr('id') + '_input-group" class="input-group">' +
 '<label class="input-group-btn" for="' + $this.attr('id') + '">' +
 '<span class="btn">' + ($this.attr('data-label') || 'New image:') + '</span>' +
 '</label>' +
 '<input id="' + $this.attr('id') + '_path" type="text" class="form-control" readonly="readonly">' +
 '<label class="input-group-btn" for="' + $this.attr('id') + '">' +
 '<span class="btn btn-primary">' +
 ($this.attr('data-button-label') || 'Browse') +
 '<div style="display:none;"></div>' +
 '</span>' +
 '</label>' +
 '</div>')
 );
 $this.appendTo('#' + $this.attr('id') + '_input-group label:last-child div:last-child');
 var $path = $('#' + $this.attr('id') + '_path', $parent).click(function(){
 $this.click();
 });
 $this.bind('change.input_file_styler', function(){
 $path.val(this.files[0] ? this.files[0].name : '');
 });
 });
 });
})(Function('return this')());

I really can't put my finger on what's wrong... but I know that something is stinking in there!

What can I improve in this code?

---

If you want to try it out:

(function(window){
	window.jQuery(function($){
		$('input[type="file"]').each(function(){
			var $this = $(this);
			var $parent = $this.parent();
			$this.detach();
			$parent.prepend(
				$('<div id="' + $this.attr('id') + '_input-group" class="input-group">' +
					'<label class="input-group-btn" for="' + $this.attr('id') + '">' +
						'<span class="btn">' + ($this.attr('data-label') || 'New image:') + '</span>' +
					'</label>' +
					'<input id="' + $this.attr('id') + '_path" type="text" class="form-control" readonly="readonly">' +
					'<label class="input-group-btn" for="' + $this.attr('id') + '">' +
						'<span class="btn btn-primary">' +
							($this.attr('data-button-label') || 'Browse') +
							'<div style="display:none;"></div>' +
						'</span>' +
					'</label>' +
				'</div>')
			);
			
			$this.appendTo('#' + $this.attr('id') + '_input-group label:last-child div:last-child');
			
			var $path = $('#' + $this.attr('id') + '_path', $parent).click(function(){
				$this.click();
			});
			
			$this.bind('change.input_file_styler', function(){
				$path.val(this.files[0] ? this.files[0].name : '');
			});
		});
	});
})(Function('return this')());

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<link href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" rel="stylesheet" type="text/css"/>
<input type="file" id="file" data-label="Select file to upload:" data-button-label="...">

Answer 1

Function('return this')()

It took two minutes to me to understand what it does. Let's see what it does step by step

Function('return this')()

This is another way of creating function. An anonymous function which simply returns this object. () at the end of it will invoke this function immediately. This is equivalent to window. Why not just use window instead of writing this mysterious statement.

Do we need IIFE?

IIFE is used to avoid polluting global scope. In the code, there is no code other than window.jQuery(function($){ inside it. Thus, IIFE is not containing any variable or function that could be global. As variables/functions defined inside a function are not accessible from outside of it(provided variables are declared using var keyword), the callback function of window.jQuery(function($){ serves as the container scope.

In simple words, IIFE can be removed safely.

jQuery(function($) {

I, generally don't use the shorthand of ready. It looks confusing for readers who are not familiar with it.

jQuery(document).ready(function() {

is readable. jQuery, call the function when document is ready.

window.jQuery

window here is not required as jQuery is global object and only jQuery will work. No harm in using window but using only jQuery will save some keystrokes.

If you're not using any other library like prototype.js, MooTools, YUI, etc. that also uses $ namespace, you can use $ instead of typing jQuery.

See Avoiding Conflicts with Other Libraries

:file selector

You may use :file pseudo-selector to select <input type="file" /> elements. This again, saves some keystrokes and is readable too.

$('input:file').each(function() {

Caching $this.attr('id')

$this.attr('id') is used 6 times in the code. This can be cached just like you cached $(this) so that, jQuery don't have to read the attribute value again and again.

var id = $this.attr('id'); // Cached the ID of element

and use id in the code.

attr() vs data()

To read the value of data-* attribute, jQuery has provided data() method. Instead of $this.attr('data-button-label'), $this.data('buttonLabel') can be used.

bind()

bind() is deprecated. To bind events us on(). bind internally calls on. By using on() directly, yo'll save one extra function call. See this answer on StackOverflow.

jQuery(document).ready(function($) {
 'use strict';
 $('input:file').each(function() {
 var $this = $(this);
 var $parent = $this.parent();
 var id = $this.attr('id');
 $this.detach();
 $parent.prepend(
 $('<div id="' + id + '_input-group" class="input-group">' +
 '<label class="input-group-btn" for="' + id + '">' +
 '<span class="btn">' + ($this.data('label') || 'New image:') + '</span>' +
 '</label>' +
 '<input id="' + id + '_path" type="text" class="form-control" readonly="readonly">' +
 '<label class="input-group-btn" for="' + id + '">' +
 '<span class="btn btn-primary">' +
 ($this.data('buttonLabel') || 'Browse') +
 '<div style="display:none;"></div>' +
 '</span>' +
 '</label>' +
 '</div>')
 );
 $this.appendTo('#' + id + '_input-group label:last-child div:last-child');
 var $path = $('#' + id + '_path', $parent).click(function() {
 $this.click();
 });
 $this.on('change.input_file_styler', function() {
 $path.val(this.files[0] ? this.files[0].name : '');
 });
 });
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<link href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" rel="stylesheet" type="text/css" />
<input type="file" id="file" data-label="Select file to upload:" data-button-label="...">

• Ismael Miguel: You made good points, but you also some I really disagree. Using Function('return this')() is just an insane safeguard, in case the window object isn't the window. Also, window.jQuery and jQuery, inside that anonymous function, is very different. Outside, anything can happen, and the window variable can have/be anything. About using :file, I agree that it is readable but it is a performance nightmare. You can read about what happens on codereview.stackexchange.com/a/101207 (I describe what happens to pseudo-selectors, like :file). [...]
• Ismael Miguel: [...] You should read the "Additional Notes" section on the :file pseudo-selector. I TOTALLY agree with "caching" $this.attr('id'). I don't know how it escaped me. For that bit alone, you deserve an upvote. But then, I disagree about your attr() vs data(). The intention is to only use attributes. Using data() can hide the name of the attributes that you can use. This exposes another issue: no documentation. But using attr() doesn't seem an issue. Also, I had no idea that bind() is deprecated. That's entirelly new to me. I will replace it with .on().
• Tushar: @IsmaelMiguel regarding the window safeguard, I don't think it is required anymore. For window.jQuery and jQuery will refer to same object as there is no local variable named jQuery in the IIFE. Only thing is that jQuery will require scope chaining till global object where it is defined. And as explained, IIFE can be removed here.
• Ismael Miguel: You're right, it isn't required anymore. Assuming your code is written into a single file and isn't minified and "mangled" with a load of other javascript files. (I think Wordpress does this, to reduce HTTP roundtrips). And those files are all wrapped in an eval() (not window.eval()!, that has a different scope) or a Function(). If one script has var window = <stuff>;, you broke the code. Try this: jsfiddle.net/tznsa59s
• Tushar: @IsmaelMiguel I didn't consider minification process when writing the answer. Your both points are valid. The review stands for the given code. Also, minification is not mentioned in the question.