2
\$\begingroup\$

My app has a feature that when you click the new data button, HTML is loaded to the page via ajax. Because I am using AJAX, all events I want to add has to be bound using the on() method.

There are 4 on('click') functions that are bound to 4 different HTML elements:

// save data
$('#container').on( {
    click: function() {
       // save code here            
    }
}, "a.save_data" );

// cancel data
$('#container').on( {
    click: function() {
       // cancel code here            
    }
}, "a.cancel_data" );

// edit data
$('#container').on( {
    click: function() {
        // edit code here            
    }
}, "a.edit_data" );

// delete data
$('#container').on( {
    click: function() {
        // delete code here
    }
}, "a.delete_data" );

Each of these functions share multiple jQuery wrappers to select certain elements within the HTML:

$(this).closest('div.content_wrap');
$(this).closest('div.Content');

Please note: The reason I am using selectors such as $(this).closest() is because the user has the ability to add the same HTML multiple times on to the page.

First, is there a better way to organize all of the on() functions? Maybe combine them into one object OR create a function?

Second, because all 4 functions use the same jQuery wrappers, is there any way to declare them in a variable globally somewhere, so i don't keep retyping them. I wasn't able to figure out how declare variables with $(this) and apply it to more than one function.

To me, how I am doing it doesn't seem very DRY and I was wondering if someone can help me better organize this.

\$\endgroup\$
  • 2
    \$\begingroup\$ See this question/answer (/shameless self-promotion) \$\endgroup\$ – Flambino Oct 20 '12 at 19:41
  • 1
    \$\begingroup\$ Hey thanks a lot, that was informative...I was able to follow, but wasn't really able to apply it to what I was doing, but I did find a way to set it up so I don't have so much repetitive code...It's under EDIT in my post if you have any additional feedback...Thanks \$\endgroup\$ – elgarcon Oct 21 '12 at 7:44
3
\$\begingroup\$

Here's how I'd probably do it (basically, it's the same as in the answer I linked to in the comments, I'm just adding it here for clarity)

var handlers = {
  save_data:   function (event, content, wrap) {
    // save code
  },

  cancel_data: function (event, content, wrap) {
    // cancel code
  },

  edit_data:   function (event, content, wrap) {
    // edit code
  },

  delete_date: function (event, content, wrap) {
    // delete code
  }
};

$('#container a').on('click', function (event) {
  var klass = this.className,
      content,
      wrap;

  if(typeof handlers[klass] === "function") {
    content = $(this).closest("div.Content"),
    wrap    = $(this).closest("div.content_wrap");
    handlers[klass].call(this, event, content, wrap);
  }

  // you want to always "absorb" the click event, you can
  // do so here or in the if-block above
  //   event.preventDefault();
});

You can write the handler functions just like you'd write them in your orginal code (i.e. this will refer to the right element, etc.). However, the click event handler will pass along the the content and wrap arguments, so you don't have to have code to select those elements in every handler function.

Of course, if your links have multiple classes, the className trick won't work. That's why I'd advocate (as I do in the answer I linked to) using a data-* attribute if possible.

\$\endgroup\$
  • \$\begingroup\$ Hey thanks for this again, much appreciated...I am actually returning back to this and I had a question...See my code above, I added a new edit...It's just a simple test...I did exactly what you have except I took out the event object, the if (typeof...) statement and the call() method...I assume the if statement was just for security sake, but I can't figure out why you need the call function, I was able to run the code without it and get the same results. Also, why did you add the event object? \$\endgroup\$ – elgarcon Oct 23 '12 at 6:14
  • \$\begingroup\$ Also, I tested this with data being pulled in via ajax and I noticed that without declaring the element I am clicking within the on() arguments, the on() functionality didn't work, but once I replaced it with live(), it did work...Which I know has been deprecated... \$\endgroup\$ – elgarcon Oct 23 '12 at 6:45
  • \$\begingroup\$ @elgarcon The typeof check is there to make sure that the handler actually exists and is a function. If you try handlers["xyz"](), and handlers["xyz"] doesn't exist or isn't a function, you'll get an error and JS just stops executing. The point of call and passing the event obj, is so the handler functions can be exactly the same as if you added them to the elements the normal way: this will be the element the event occurred on, and the 1st argument is the event obj. \$\endgroup\$ – Flambino Oct 23 '12 at 8:03
  • \$\begingroup\$ @elgarcon Not sure what you mean by the on vs live stuff. But if things stop working when you remove things, well, don't remove things. jQuery's on function works, so the problem must be on your end. Don't know what else to tell ya. \$\endgroup\$ – Flambino Oct 23 '12 at 8:11
1
\$\begingroup\$

Perhaps something like this?

function manipulateData($action){
    $('#container').on( {
        click: function() {
            switch(action){
                case "save" : saveData();
                    break;
                case "delete" : deleteData();
            }
        }
    }, "a." + action + "_data" );
}

Also, see Flambinos comment

\$\endgroup\$
  • 1
    \$\begingroup\$ Hey thanks for taking the time to help...I came up with something similar and posted it in my post under EDIT...My thinking there is, because I am binding 4 different functions to 4 different elements, I just wanted a way to cut back on the repetitive code. What I have done allows me to use the same variables for all events and also allows to write 'click' and 'return false' only once ...Any improvements to this that you know of? \$\endgroup\$ – elgarcon Oct 21 '12 at 7:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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