6
\$\begingroup\$

I have just created my first jQuery plugin. I am looking to suggestions on how to improve it, leading to a code optimization.

(function($) {
   var plugin_name = 'W.jQuery.FieldEvents';
   var name_space = 'wFieldEvents';  
   var methods = {
      init : function(options) { 
         return this.each(function(){
            var field = $(this);
            var settings = {
               'edit_button' : '',
               'submit_button':'',
               'error_message':'value not found',
               callback:function(){
                  alert('no call back defined for'+field.attr('id'))
               }
            };
            settings = $.extend(settings, options);
            var data = field.data(name_space);
            var edit_button = $(settings.edit_button);
            var submit_button = $(settings.submit_button);
            if (!data) {
               $(this).data(name_space, {
                  submit_button : submit_button,
                  edit_button :edit_button,
                  callback:settings.callback,
                  error_message:settings.error_message
               });
            }
            methods.enable_submit.call(field); 
         });
      },
      disable_edit: function(){
         return this.each(function(){
            var field = $(this);
            var data = field.data(name_space);
            //alert(data.edit_button.attr('id'));
            data.edit_button.attr('disabled','disabled').off('click.'+name_space).addClass('ui-state-disabled');
         });
      },
      enable_submit : function()
      {
         return this.each(function(){
            var field = $(this);
            var data = field.data(name_space);
            //alert('working');
            //alert('enabled submit called for '+field.attr('id'));
            methods.disable_edit.call(field);
            field.removeAttr('disabled');
            data.submit_button.removeAttr('disabled').removeClass('ui-state-disabled');
            data.submit_button.on('click.'+name_space,function(){
               methods.submit.call(field);
            });
            field.focus();
         });
      },
      enable_edit: function()
      {
         return this.each(function(){
            var field = $(this);
            var data = field.data(name_space);
            field.attr('disabled','disabled');
            data.edit_button.removeAttr('disabled').removeClass('ui-state-disabled');
            data.edit_button.on('click.'+name_space,function(){
               methods.enable_submit.call(field);
            });
         });
      },
      disable_submit: function(){
         return this.each(function(){
            var field = $(this);
            var data = field.data(name_space);
            field.attr('disabled','disabled');
            data.submit_button.attr('disabled','disabled').off('click.'+name_space).addClass('ui-state-disabled');
            methods.enable_edit.call(field);
         });
      },
      show_error: function(){
         return this.each(function(){
            var field = $(this);
            var data = field.data(name_space);
            //alert(data.error_message);
            field.focus();
            $(dialog_id).html(data.error_message);
            $(dialog_id).dialog('open');
         });
      },
      submit:function(){
         return this.each(function(){
            //alert('submit called');
            var field = $(this);
            var data = field.data(name_space);
            var value = field.val();
            if(value != '')
            {
               methods.disable_submit.call(field);
               data.callback();
            }
            else
            {
               methods.show_error.call(field);
            }
         });
      },
      destroy : function() {
         return this.each(function(){
            var $this = $(this),
            data = $this.data(name_space);
            // Namespacing FTW
            $(window).W.jQuery.FieldEvents(name_space);
            data.wFieldEvents.remove();
            $this.removeData(name_space);
         });
      }
   };

   $.fn.W_FieldEvents  = function( method ) {

      // Method calling logic
      if ( methods[method] ) {
         return methods[ method ].apply( this, Array.prototype.slice.call( arguments, 1 ));
      } else if ( typeof method === 'object' || ! method ) {
         return methods.init.apply( this, arguments );
      } else {
         $.error( 'Method ' +  method + ' does not exist on '+plugin_name );
         return false;
      }    

   };
})(jQuery);

Usage example:

$(document).ready(function(){

   $('#po_number').W_FieldEvents({
     'submit_button' : '#po_number_submit',
     'edit_button'   : '#po_number_edit',
     'error_message' : 'PO# number not found',
     callback        : function(){
       alert('submit done');
     }
   });

});
\$\endgroup\$
5
  • 3
    \$\begingroup\$ It's always useful to run the code through jslint or jshint. For instance, both caught that dialog_id is not defined anywhere but used in show_error(). You also have an implicit global (data in destroy()) which neither jshint or jslint caught (I just spotted it by chance). \$\endgroup\$
    – Flambino
    Commented Oct 11, 2012 at 3:06
  • \$\begingroup\$ What about data management, am i doing it correctly? is this good approach to save data for each element in its jQuery data attribute. Is there a way to store elements data in a single datastore? I have also noticed that if i call this plugin twice on an element it will double map the events. i.e showing two error messages instead of one. i want to avoid it, is there any way out? \$\endgroup\$
    – Waqas Ali
    Commented Oct 11, 2012 at 15:04
  • 1
    \$\begingroup\$ You could store everything in a closure, I suppose, but I see no reason to not use .data(). Seems pretty straightforward to me (disclaimer: I've never had to make a jQuery plugin, so perhaps someone with more experience has a different opinion). As for the double events, I'd suggest you check for existing data on the elements. If it's there, it's been set up before, and you can merge the existing settings and the passed-in options, instead of merging with the default options. Then calling it twice on the same element won't double the event, but will update the settings. \$\endgroup\$
    – Flambino
    Commented Oct 11, 2012 at 16:10
  • 1
    \$\begingroup\$ Waqasalieee, you don't need name_space as well as plugin_name. Simply use the plugin_name as the namespace name, and start the supervisor with $.fn[plugin_name] = function(method) {. @Flambino, with this pattern, you can only store whole-plugin stuff in the closure; no closure within the pattern is scoped to be element-specific so data must be stored in the DOM with .data(). \$\endgroup\$ Commented Oct 11, 2012 at 19:48
  • \$\begingroup\$ Beetroot, if am using this plugin name ' W.jQuery.FieldEvents ' then it contains dots (.), i thought it will create problems in event binding code i.e : edit_button.on('click.'+name_space,function(){//code}); \$\endgroup\$
    – Waqas Ali
    Commented Oct 12, 2012 at 11:13

1 Answer 1

3
\$\begingroup\$

You can refactor the init function.

init : function(options) {
         var defaults = {
           'edit_button' : '',
           'submit_button':'',
           'error_message':'value not found'
          };
          options = $.extend({}, defaults, options);
         return this.each(function(){
            var field = $(this);
            if (!options.hasOwnProperty('callback') ) {
              options.callback = function(){
                alert('no call back defined for'+field.attr('id'))
              }
            }             
            var data = field.data(name_space);
            var edit_button = $(options.edit_button);
            var submit_button = $(options.submit_button);
            if (!data) {
               $(this).data(name_space, {
                  submit_button : submit_button,
                  edit_button :edit_button,
                  callback:options.callback,
                  error_message:options.error_message
               });
            }
            methods.enable_submit.call(field);
         });
      },

As Flambino mentioned run your code through JSlint

\$\endgroup\$

Your Answer

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

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