Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have been writing a jQuery widget for a couple of days now. Originally I needed this functionality for something I am writing at work, but I felt like I could abstract it a little more so here I am. I think it's about finished for now, I may add some features depending on how It gets used (if it does).

This is my first jQuery widget and I have been programming in JavaScript for just over a month now, so I don't think the code will be particularly great, but I've tried my hardest.

I have done some vague testing and it does seem to work in all browsers.

My main gripes are validation of the options, of which I don't really know how to do, however, I am going to do some proper research on this and come back to it.

There a few other problems with it but I think that this is not the right place to ask about those, so will leave it for now.

I would just like some feedback on my coding, how to improve/do it better, now and in the future. It's quite a big chunk of code but I didn't see anything in the FAQ about length of code.

I've also attached a jsFiddle so you can mess with it: http://jsfiddle.net/jCVzp/2/

I've tried to comment as best as I can and have noted all the options in the file description, and more guidance needed, just ask.

All the CSS is present in the jsFiddle, didn't think it was necessary to post it here.

Any time and help is greatly appreciated.

(function($) {
            /* 
            *   Document   : list-widget-0.1
            *   Created on : 06-Aug-2012, 20:53:49
            *   Author     : aydin hassan
            *   Description:
            *       This widget is designed to be attatched to a div
            *       
            *       Using the options you can specify one of three list types:
            *           1.) a normal list if items (normal)
            *           2.) a list of items with a cross/tick img depending on its status (act-deact)
            *               ---Clicking the image will toggle its status (change the img and class) and run a provided callback
            *           3.) a list of items with a trash can icon (remove)
            *               ---Click the image will run a supplied calback and then remove the item
            *
            *       Each of the lists has various callbacks:
            *           1.) The global click callback (selectabble.clickCallBack), if passed, will launch on clicked an element for any list type
            *           2.) The global click off callback (selectable.clickOffCallBack), if passed, will launch when an item is clicked off.
            *               ---Works in conjuction with selectable.sticky - if this is set to true elements will stay higlighted until they are clicked off
            *               ---NOTE: click off callback will not be processed if selectable.sticky is set to true
            *           3.) The act-deact callback, if passed, will run after a user has clicked the img, or, if there is no global click callback (selectabble.clickCallBack) it will 
            *               run when the li element is clicked
            *           4.) The remove callback, if passed, will run after the user clicks on the trash-can icon
            *           5.) The add button callback, if passed, will run when a user clicks the add button
            *
            *       List data:
            *           The list data must be an object of the following form:
            *               act-deact:
            *               var data = {
            *                   "0":{"name":"list item 1","activated":true},
            *                   "1":{"name":"list item 2","activated":true},
            *                   "2":{"name":"list item 3","activated":false}
            *               };
            *               
            *               normal/remove:
            *               var data = {
            *                   "0":{"name":"list item 1"},
            *                   "1":{"name":"list item 2"},
            *                   "2":{"name":"list item 3"}
            *               };
            *       
            *       Other options:
            *           title:          ---The title of the list box
            *           description:    ---Some description of the data
            *           searchBar:      ---BOOL - To display a filter box
            *           addButton:      ---BOOL - To display an add button
            *           idStyle:        ---A string to prepend to the id of the item id in the data object to use as the li element id attribute
            *           
            */      

          $.widget( "widgets.adminList", {


            // These options will be used as defaults

            options: { 
              list: null,
              title: "Title",
              description: "Some description",
              searchBar: true,
              addButton: false,

              addCallBack: null,
              listType: "normal",
              idStyle: "li-id-",
              removeCallBack: null,
              toggleCallBack: null,
              selectable: null    

            },


            // Set up the widget
            _create: function() {

            },

            _init: function() {
                var options = this.options;
                //Build the container elements
                this._buildTableHead();

                //switch type of list and run setup function for specified type
                switch (options.listType) {
                    case "normal": this._setUpNormal();
                        break;
                    case "act-deact": this._setUpActDeact();
                        break;
                    case "remove": this._setUpRemove();
                        break;
                }


                //If the selectable options are configured then we may want sticky elements (hover css stays until clciked again, or another elements is clicked)
                //And there may also be click on and click off callbacks
                if(options.selectable != null) {
                    $(".list-widget-list-container .list-widget-list li a").live('click',(function(e) {
                        //prevent the href("") from being followed, href is needed for IE to display hover
                        e.preventDefault();
                        //If we want elements to stick
                        if(options.selectable.stick) {
                            //If this element is stuck already
                            //unstick it and run click off call back if supplied
                            if($(this).hasClass("selected")) {
                                $(this).removeClass("selected");
                                if(options.selectable.clickOffCallBack != null){
                                    options.selectable.clickOffCallBack(this);
                                }
                            //Else unstick any other elements
                            //Stick this element
                            //and run click callback
                            } else {
                                $(".list-widget-list-container .list-widget-list").find(".selected").removeClass("selected");
                                $(this).addClass("selected");
                                if(options.selectable.clickCallBack != null){
                                    options.selectable.clickCallBack(this);
                                }
                            }
                        //If we don't want to stick just run click on callback if supplied
                        } else {
                            if(options.selectable.clickCallBack != null){
                                options.selectable.clickCallBack(this);
                            }
                        }
                    }));
                }

                //If add callback is supplied
                //create click event
                if(options.addButton && options.addCallBack != null) {
                    $(".list-widget-head-div .header-table .add-but").click(function() {
                        options.addCallBack(this);
                    });
                }


                //create event for filter box
                var that = this;                    
                $(".list-widget-head-div .header-table .list-search-box").bind("keyup",function(){
                    that._listFilter(this);
                });

                //Add the list elements
                this.add(options.list);
            },

            _listFilter:function(e){
                var filter = $(e).val(); // get the value of the input, which we filter on
                var list = $(".list-widget-list-container .list-widget-list");

                //only filter if value exists
                if(!filter || filter.length < 1 ){
                    list.find("li").show();
                }
                else{
                    list.find("li").each(function(){
                        //if a element text contains the filter value, hide it
                        if($("a", this).text().toLowerCase().indexOf(filter.toLowerCase()) == -1  ){
                            $(this).hide();
                        }
                        else{
                            //else show it
                            $(this).show();
                        }
                    });
                }
            },

            _buildTableHead:function(){
                //get the current options
                var options = this.options;
                //get the div the widget has been attatched to
                var self = this.element;

                //build table header
                var headCells = $("<tr />")
                        .append($('<td  />')
                            .attr("width","33%")
                            .attr("height","45px")
                            .append($('<p />')
                                .addClass("title")
                                .text(options.title)
                            )
                        );

                //depending on the options;
                //add a search bar and add button

                //if we want searchbar and add button..
                if(options.searchBar && options.addButton) {
                    headCells.append($('<td  />')
                        .attr("width","33%")
                        .attr("height","45px")
                        .attr("text-align","center")
                        .append($('<img />')
                            .addClass("add-but")
                            .attr("src","add-image.png")
                        )
                    )
                    .append($('<td  />')
                        .attr("width","33%")
                        .attr("height","45px")
                        .append($('<input />')
                            .attr("type","text")
                            .addClass("list-search-box")
                        )
                    );
                //if we only want add button
                } else if (!options.searchBar && options.addButton) {
                    headCells.append($('<td  />')
                        .attr("width","50%")
                        .attr("align","center")
                        .append($('<img />')
                            .addClass("add-but")
                            .attr("src","add-image.png")
                        )
                    );
                //if we only want search bar
                } else if (options.searchBar && !options.addButton) {
                    headCells.append($('<td  />')
                        .attr("width","50%")
                        .append($('<input />')
                            .attr("type","text")
                            .addClass("list-search-box")
                        )
                    );
                }

                //Build the tables and append the cells
                self.append($('<div />')
                        .addClass("list-widget-head-div")
                        .addClass("ui-corner-top")
                        .append($('<table />')
                            .addClass("header-table")
                            .append(headCells)
                        )
                        //Add the description if present
                        .append($("<p />")
                            .addClass("descript")
                            .text(options.description)
                    )
                );

                //Add the actual list element
                self.append($("<div />")
                    .addClass("list-widget-list-container")
                    .addClass("ui-corner-bottom")
                        .append($("<ul />")
                        .addClass("list-widget-list")
                    )
                );
            },

            _setUpNormal:function(){
                //get the current options
                var options = this.options;

            },
            _setUpActDeact:function(){
                //get the current options
                var options = this.options;

                //click event upon clicking the item image
                //Changes the status and can be provided with a call back
                //which has access to the clicked element


                //If there is no clickCallBack supplied, we apply the toggle action to the whole li element
                if(options.selectable.clickCallBack == null) {
                    $(".list-widget-list-container .list-widget-list li").live('click',function(e){
                        //get the clicked element
                        var imgElem = $(this).find("img.list-img");
                        //var to store status
                        var active;
                        //get status of item
                        //and remove the class
                        if(imgElem.hasClass("deactivated")) {
                            active = false;
                            imgElem.removeClass("deactivated");
                        } else {
                            active = true;
                            imgElem.removeClass("activated");
                        }
                        //add the loading class
                        imgElem.addClass("loading");
                        imgElem.attr("src","ajax-loader.gif")
                        //if callback has been provided, run it
                        if(options.toggleCallBack != null) {
                            options.toggleCallBack(this)
                        }
                        //remove loading class
                        imgElem.removeClass("loading");
                        //add the oposite class(item has been toggled)
                        if(active) {
                            imgElem.addClass("deactivated");
                            imgElem.attr("src","cross.png")
                        } else {
                            imgElem.addClass("activated");
                            imgElem.attr("src","tick.png")
                        }
                    });
                //if there is a clickCallBack then we just apply the toggle event to the image
                } else {
                    $(".list-widget-list-container .list-widget-list li img.list-img").live('click',function(e){
                        //get the clicked element
                        var imgElem = $(this);
                        //var to stored status
                        var active;
                        //get status of item
                        //and remove the class
                        if(imgElem.hasClass("deactivated")) {
                            active = false;
                            imgElem.removeClass("deactivated");
                        } else {
                            active = true;
                            imgElem.removeClass("activated");
                        }
                        //add the loading class
                        imgElem.addClass("loading");
                        imgElem.attr("src","ajax-loader.gif")
                        //if callback has been provided, run it
                        if(options.toggleCallBack != null) {
                            options.toggleCallBack(this)
                        }
                        //remove loading class
                        imgElem.removeClass("loading");
                        //add the oposite class(item has been toggled
                        if(active) {
                            imgElem.addClass("deactivated");
                            imgElem.attr("src","cross.png")
                        } else {
                            imgElem.addClass("activated");
                            imgElem.attr("src","tick.png")
                        }
                    });
                }
            },
            _setUpRemove:function(){
                //get the current options
                var options = this.options;

                //click event upon clicking the delete image
                //runs a provided callback and the removes item
                $(".list-widget-list-container .list-widget-list li img.list-img").live('click',function(e){
                    //if callback has been provided, run it
                    if(options.removeCallBack != null) {
                        options.removeCallBack(this)
                    }
                    //remove element
                    $(this).parent().remove();
                });
            },

            // Use the _setOption method to respond to changes to options

            _setOption: function( key, value ) {

              switch( key ) {

                case "clear":
                  // handle changes to clear option

                  break;

              }

              // In $ UI 1.8, you have to manually invoke the _setOption method from the base widget

              $.Widget.prototype._setOption.apply( this, arguments );
              // In $ UI 1.9 and above, you use the _super method instead
              this._super( "_setOption", key, value );

            },

            // Use the destroy method to clean up any modifications your widget has made to the DOM

            destroy: function() {

              // In $ UI 1.8, you must invoke the destroy method from the base widget

              $.Widget.prototype.destroy.call( this );
              // In $ UI 1.9 and above, you would define _destroy instead of destroy and not call the base method

            },

            /* This fuction when presented with an object in the following format
             * var data = {
             *      "0":{"name":"list item 1","activated":true},
             *      "1":{"name":"list item 2","activated":true},
             *      "2":{"name":"list item 3","activated":false}
             *  };
             * Will append the items to the bottom of the list
             */ 

            add: function(list) {
                //Pick add function depending on listType
                switch(this.options.listType) {
                    case "normal":this._addNormal(list);
                        break;
                    case "act-deact":this._addActDeact(list);
                        break;
                    case "remove":this._addRemove(list);
                        break;
                }
            },

            _addNormal: function(list) {
                var options = this.options;

                //Get the list elemenent
                var ulElem = $(".list-widget-list-container .list-widget-list");

                //For each list item
                $.each(list, function(key,val){
                    //Append the item to the list
                    ulElem.append($("<li />")
                        .attr("id",options.idStyle + key)
                            .append($("<a />")
                            .text(val.name)
                        )
                    );
                });
            },

            _addActDeact: function(list) {
                var options = this.options;

                //Get the list elemenent
                var ulElem = $(".list-widget-list-container .list-widget-list");

                //For each list item
                $.each(list, function(key,val){
                    //creat li element
                    var li = $("<li />")
                        .attr("id",options.idStyle + key)
                            .append($("<img />")
                                .addClass("list-img")
                            )
                            .append($("<a />")
                                .text(val.name)
                                .attr("href","")
                                .addClass("actdeact")
                        );
                    //add activated/deactivated class
                    //depending on item status 
                    if(val.activated === true) {
                        li.find("img").addClass("activated");
                        li.find("img").attr("src","tick.png");
                    } else {
                        li.find("img").addClass("deactivated");
                        li.find("img").attr("src","cross.png");
                    }
                    //Append the list item to the list
                    ulElem.append(li);
                });

            },

            _addRemove: function(list) {
                var options = this.options;

                //Get the list elemenent
                var ulElem = $(".list-widget-list-container .list-widget-list");

                //For each list item
                $.each(list, function(key,val){
                    //Append the item to the list
                    ulElem.append($("<li />")
                        .append($("<img />")
                                .addClass("list-img")
                                .attr("src","trash_can.png")
                            )
                        .attr("id",options.idStyle + key)
                            .append($("<a />")
                                .text(val.name)
                                .addClass("remove")
                        )
                    );
                });
            },

            //Function to empty the list
            empty: function() {
                $(".list-widget-list-container .list-widget-list").empty();
            }

        });
share|improve this question
1  
Just skimmed through it, and it seems well-structured and nicely commented. Kudos! I also tried running it through jshint which found a few things like missing semicolons and such. Try it out; it's a good way to catch the small stuff. Oh, and for checking callbacks, use typeof someCallback === 'function' rather than someCallback != null. –  Flambino Nov 13 '12 at 10:49
    
Thankyou, I will make those changes and checkout jshint! –  Aydin Hassan Nov 13 '12 at 11:02
add comment

1 Answer

From a once over:

  • Do not do this:

    if(options.selectable != null)
    

    do either

    if(options.selectable)
    

    or

    if(options.selectable !== null) //Notice the ==
    
  • You are not consistent with semicolons, use jshint.com to fix your code
  • Extract some constants out, particularly widths and heights in your element building code
  • You use both single and double quotes to delimit strings, pick one, preferably single quotes
  • Considering using $().toggleClass(), you seem to be toggling a few times classnames yourself
  • I would allow toggleCallBack to cancel the toggeling so that I could use it validate the toggle

Other than that I think your code is solid.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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