patternjavascriptMinor
Transforming a div to a widget
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:
`(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
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:
can be put outside the
secondly as a matter of tidyness:
is redundant. Either use
EDIT:- I've noticed another tidyness issue:
you don't need to redeclare the options just use it.
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.
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.