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

Transforming a div to a widget

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

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 = $('')
.addClass('ui-widget-header widgethead')
.css({'min-width':'130px'});

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

//whats the best place to put this fun

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.

Code Snippets

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
        };
(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.
var options = $.extend(defaults, options);
var o=options;
widgetIt: function(options){
    var options = //...
widgetIt: function(options){
    options = //...

Context

StackExchange Code Review Q#5005, answer score: 4

Revisions (0)

No revisions yet.