HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Bootstrap file input styler

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bootstrapinputfilestyler

Problem

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

Simply, it turns this:

Into this input:

(function(window){
    window.jQuery(function($){
        $('input[type="file"]').each(function(){
            var $this = $(this);
            var $parent = $this.parent();
            $this.detach();
            $parent.prepend(
                $('' +
                    '' +
                        '' + ($this.attr('data-label') || 'New image:') + '' +
                    '' +
                    '' +
                    '' +
                        '' +
                            ($this.attr('data-button-label') || 'Browse') +
                            '' +
                        '' +
                    '' +
                '')
            );

            $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(
$('' +
'' +
'' + ($this.attr('data-label') || 'New image:') + '' +
'' +
'' +
'' +
'' +
($this.attr('data-button-label') || 'Browse') +
'' +
'' +
'' +
'')
);

$this.appendTo('#' + $this.attr('id') + '_input-group label:last-child div:last-child');

var $path = $('#' + $this.attr('id') + '_path', $parent).click(function(){
$this.click

Solution

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 ` 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(
$('' +
'' +
'' + ($this.data('label') || 'New image:') + '' +
'' +
'' +
'' +
'' +
($this.data('buttonLabel') || 'Browse') +
'' +
'' +
'' +
'')
);

$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 : '');
});
});
});


`

Code Snippets

Function('return this')()
jQuery(document).ready(function() {
$('input:file').each(function() {
var id = $this.attr('id'); // Cached the ID of element

Context

StackExchange Code Review Q#155244, answer score: 3

Revisions (0)

No revisions yet.