3
\$\begingroup\$

I was wondering if you could review the following code I've written as a polyfill for the placeholder attribute in HTML5?

The object of the polyfill is to replicate the functionality of the placeholder attribute. In effect, the polyfill is supposed to do the following:

  1. Default text to be shown.
  2. On Focus: Erase the default text.
  3. On Focus Out: Put in default text if nothing has been entered

Dependancies:

  1. jQuery

    var Utils = Utils || {};
    
    Utils = (function (window, $) { 
    
        var _c, _private, _public;
    
        _private = {
    
            /**
             * @event
             * @param e The event object.
             */
            'handleFocusIn': function (e) {
                var $target = $(e.currentTarget);
                if($target.val() === $target.attr('placeholder')) {
                    $target.val('');
                }
                $target
                    .unbind('focusin')
                    .bind('focusout', _private.handleFocusOut);
            },
    
            /**
             * @event
             * @param e The event object.
             */
            'handleFocusOut': function (e) {
                var $target = $(e.currentTarget);
                if($target.val() === '') {
                    $target.val($target.attr('placeholder'));
                }
                $target
                    .unbind('focusout')
                    .bind('focusin', _private.handleFocusIn);
            }
        };
    
        _public = {
            /**
             * @function
             * @description
             * A poly-fill for the placeholder attribute.
             */
            'supportsPlaceholderAttr': function (enableFallback) {
                var placeHolderSupported = false;
    
                //If enableFallback is either undefined or not a Boolean, set it to false
                if((typeof enableFallback === 'undefined') || !(enableFallback instanceof Boolean)) {
                    enableFallback = false;
                }
    
                //if placeholder is supported, no reason to continue on
                if ('placeholder' in document.createElement("input")) {
                    return placeHolderSupported = true;
                }
    
                if (!placeHolderSupported && !!enableFallback) {
                    var $inputEle;
                    $('input')
                        .each( function(index, ele) {
                            $inputEle = $(ele);
                            if($inputEle.attr('placeholder')) {
                                $inputEle
                                    .val($inputEle.attr('placeholder'))
                                    .bind('focusin', _private.handleFocusIn);
                            }
                        });
                }
            }
        };
    
        return _public;
    
    }(window, jQuery));
    
\$\endgroup\$
8
  • \$\begingroup\$ fixed \$\endgroup\$
    – Raynos
    Nov 8, 2011 at 16:57
  • \$\begingroup\$ @Raynos I liked how you shortened up the code, however what if I wanted to extend shim to have more than just the polyfill for the placeholder attribute? Also, there are no checks for the enableFallback parameter - shouldn't best practices have you check for that value? \$\endgroup\$ Nov 8, 2011 at 17:02
  • \$\begingroup\$ mocked \$\endgroup\$
    – Raynos
    Nov 8, 2011 at 17:07
  • \$\begingroup\$ I checked with enableFallback && run loop. i used a short circuiting trickery to make sure you only bunch the focusin/out handlers in if the enableFallback flag is set \$\endgroup\$
    – Raynos
    Nov 8, 2011 at 17:08
  • 1
    \$\begingroup\$ @alvincrespo Could you elaborate on or link to some information regarding "the nature of IE7 mobile"? \$\endgroup\$
    – Ryan Kinal
    Nov 8, 2011 at 17:36

1 Answer 1

3
\$\begingroup\$

Note: Regarding the OP's comment on the "nature of IE7 mobile", I suspect it has more to do with jQuery's implementation of focusin and focusout. This may not be the case, but I have written my review with that assumption in mind.

There are a few things about your code that are, to put it bluntly, pointless.

First, checking whether Utils exists is a good thing, but you then immediately clobber it with the return of your function:

var Utils = Utils || {};

Utils = (function(window, $) {
    /* ... */
    return _public;
})(window, jQuery);

Something like the following would most likely be better:

var Utils = Utils || (function(window, $) {
    /* ... */
    return _public;
})(window, jQuery);

Second, why pass window to the function at all? Browsers will not allow overwriting of the window object, and your parameter name is window anyway, so it doesn't save you anything.

Third, _private is unnecessary. Any variables (including functions) that you don't include in whatever object you return will be hidden by the local scope of your self-executing function.

var handleFocusIn = function(e) { /*...*/ },
    handleFocusOut = function(e) { /*...*/ };

_public is also unnecessary, though not as much as _private. If it were me, I would simply return the object literal, rather than bothering with the _public variable.

Lastly, given that your elements will always be input elements, there is no need to use focusin and focusout; Simply use focus and blur - this will avoid any possible bubbling issues, and then, most importantly, allow you to dispense with the unbinding/rebinding that happens in your handlers.

@Raynos also makes some good points on your implementation of supportsPlaceHolderAttr, regarding variables and conditionals.

//if placeholder is supported, no reason to continue on
if ('placeholder' in document.createElement("input")) {
    // useless variable, just return true
    return placeHolderSupported = true;
}

// useless placeHoldSupported check, always false. Useless enableFallback conversion always a bool
if (!placeHolderSupported && !!enableFallback) {

It's a good attempt at writing unobtrusive code, but it falls short of good modularity and namespacing. Still, like I said, a good start.

\$\endgroup\$
4
  • 3
    \$\begingroup\$ Passing window into an anoymous closure is a tiny micro optimisation (one less layer of lookup) and allows you to minify the window variable. \$\endgroup\$
    – Raynos
    Nov 8, 2011 at 18:05
  • \$\begingroup\$ Point taken. +1 \$\endgroup\$
    – Ryan Kinal
    Nov 8, 2011 at 18:06
  • \$\begingroup\$ This is exactly what I am looking for. A clear explanation of why my code doesn't really make sense - if it doesn't. Thanks so much @RyanKinal It was clear and to the point and definitely helps me improve. Any more answers/suggestions would be great. :) \$\endgroup\$ Nov 8, 2011 at 18:09
  • \$\begingroup\$ I added a suggestion on the idea of detecting whether Utils exists. \$\endgroup\$
    – Ryan Kinal
    Nov 8, 2011 at 18:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.