Transforming a div to a widget

By Posted on

Problem

My first jQuery plugin is working, but I wonder if my coding is conventional. What can I do better and how can I make it more efficient?

I know it’s only a small plugin, so efficiency is not a big deal, but I don’t want to start with bad habits.

What this plugin is doing:

  • It transforms a div to a widget (with header and content)
  • When double-clicked on header, content fades in/out (depending on current state open or closed)
  • Load content via Ajax when opened
  • Has option for reloading content when opened again
  • Has option for refreshing content when opened
(function($){
    $.fn.extend({

        widgetIt: function(options) {

            var defaults = {
                title: 'Widget Title',
                load:'',
                top: '50px',
                left: '400px',
                width: '500px',
                afterLoad: function(){},
                reload:false, //if true the content gets reloaded everytime widget opens
                refresh:false //if set to (example) 3000, the content gets refreshed when widget is open
            };

            var options = $.extend(defaults, options);
            var o=options;
           //conditional manipulate options
           if(o.refresh){o.reload=true}//when refresh is set, reload is true


            return this.each(function() { 

                  var container=$(this).css({'z-index':3, display:'inline-block',position:'absolute',top:o.top,left:o.left,'max-width':o.width})
                                .addClass('widget_container');
                  var header = $('<div></div>')
                                .addClass('ui-widget-header widgethead')
                                .css({'min-width':'130px'});

                  var title =$('<div></div>').addClass("w_tit").html(o.title);
                  var content =$('<div></div>')
                               .addClass("w_content")
                               .hide();
                 var timer = null;

            //whats the best place to put this function?
            //is it good to have it this in each function?                

            function loadData(){
                              $.ajax({
                            url: o.load,
                            context: content,
                            success: function(data){
                            $(content).html(data);
                            reload=false;
                            //[hide ajax spinner]
                            if(o.refresh){
                            timer=setTimeout(loadData,o.refresh)    ;}                      
                            o.afterLoad.call();
                            },
                            error: function(){
                            // error code here
                            }
                            });
            }



                  //append
                  $(title).appendTo(header) ;
                  $(header).appendTo(container) ;
                  $(content).appendTo(container) ;

                  //make draggable
                  $(container).draggable({
                    cancel: 'input,option, select,textarea,.w_content',
                    opacity: 0.45,
                    cursor: 'move'
                    });

                  //cbinding
                    var display=$(content).css('display'); //check if widget is open=block or closed=none


                    var reload=true ; //set initially to true->reload content every time widget opens

                  $(header).dblclick(function(){

                    $(content).fadeToggle();//first open or close widget
                    //[show ajax spinner]

                        if(display="block" && reload){//only load on widget open event
                            loadData();
                        }else if(display="none"){reload=o.reload;clearTimeout(timer);}//set reload true or false
                  });


                 $(header).click(function (){
                    $(container).topZIndex('.widget_container');

                 });
                 //close all open widgets and animate back to original position
                 $('#deco').click(function (){
                            $(content).hide();
                            $(container).animate({ "left": o.left, "top": o.top}, "slow");
                 });  
            });
        }
    });
})(jQuery);

Solution

Only two small points of improvement:

var defaults = {
            title: 'Widget Title',
            load:'',
            top: '50px',
            left: '400px',
            width: '500px',
            afterLoad: function(){},
            reload:false, //if true the content gets reloaded everytime widget opens
            refresh:false //if set to (example) 3000, the content gets refreshed when widget is open
        };

can be put outside the widgetIt function. I do this as a standard as (in theory) it doesn’t get created each time. The difference in reality is probably indistinguishable

(function($){
    var defaults = {
            title: 'Widget Title',
            load:'',
            top: '50px',
            left: '400px',
            width: '500px',
            afterLoad: function(){},
            reload:false, //if true the content gets reloaded everytime widget opens
            refresh:false //if set to (example) 3000, the content gets refreshed when widget is open
        };
    $.fn.extend({
         // etc.

secondly as a matter of tidyness: 

var options = $.extend(defaults, options);
var o=options;

is redundant. Either use options or o but don’t have both as this can get confusing. Also I wouldn’t use single letter variable names for anything other than i. Being Explicit about the variable leads to an easier read in 6 months when you come back to it.

EDIT:- I’ve noticed another tidyness issue:

widgetIt: function(options){
    var options = //...

you don’t need to redeclare the options just use it.

widgetIt: function(options){
    options = //...

Just as a help I often paste my code into http://jsfiddle.net and click “JSLint”. Its not always correct (as I don’t always paste all my code in) but it helps a lot. Also the “TidyUp” buttons is awesome too.

  • Typically, jQuery plugins follow the jQuery Core Style guidelines. This has a number of benefits, including that it’s easier for potential savvy users to proof-read/review the plugin before using it, extend the plugin, or incorporate it into some larger project (e.g. jQuery UI). A more verbose explanation is available in slides 18-27 of this presentation.
  • You may wish to return this from your widgetIt method to allow chaining (for consistency with the rest of the jQuery API).
  • You may want to expose defaults in a way that allows users to override it (a la $.datepicker.setDefaults).
  • It seems like there might be a bug in $.extend(defaults, options); I believe that modifies your default settings. I’ve usually seen it written $.extend({}, defaults, options).

For more plugin best practices, you might want to take a look at the “slides” from the presentation Ben Alman delivered at this year’s jQuery Boston conference.

I see a little bug on lines 85 – 85:

if(display="block" && reload){//only load on widget open event
     loadData();
}
else if(display="none"){
     reload=o.reload;clearTimeout(timer);
 }//set reload true or false

Here you are setting the value of a variable called display instead of evaluating its value. The && operator JavaScript may work different than you expect. It evaulates the first expression and returns the result of it if its true, the first is false it returns the result of the second expression. So this means line 83 will ALWAYS return true since anything that doesn’t evaluate to FALSE in JavaScript is true, like setting the value of a variable.

For feedback on your code, I’d group your var statements together instead of declaring them separately. For example: 

var container=$(this).css({'z-index':3, display:'inline-block',position:'absolute',top:o.top,left:o.left,'max-width':o.width})
              .addClass('widget_container'),
header = $('<div></div>')
              .addClass('ui-widget-header widgethead')
              .css({'min-width':'130px'}),
title =$('<div></div>').addClass("w_tit").html(o.title),
content =$('<div></div>')
             .addClass("w_content")
             .hide(),
timer = null;

In the blow lines you are doubling jQuery:

//append
$(title).appendTo(header) ;
$(header).appendTo(container) ;
$(content).appendTo(container) ;

This could be written like:

//append
title.appendTo(header) ;
header.appendTo(container) ;
content.appendTo(container) ;

When you created the objects, they already have jquery attached to them.

Leave a Reply

Your email address will not be published. Required fields are marked *