sourcebitsllc / chocolatechip-ui

Mobile Web App Framework
www.chocolatechip-ui.com
MIT License
617 stars 88 forks source link

$.UIDeletable cannot read property 'on' of undefined #73

Closed jeff183 closed 10 years ago

jeff183 commented 10 years ago

i have a list loaded thru ajax that i'm using with $.UIDeletable. I put $.UIDeletable in a function called initDeletables (as suggested). I start with an empty list. I append one item. no problem. The second item i append, i get the following error: Uncaught TypeError: Cannot read property 'on' of undefined chui-3.5.5.js:1459

I do a work around by putting $.UIDeletable in a try catch phrase, and just ignore the error.

sourcebits-robertbiggs commented 10 years ago

Append all your items. When done, then only init the delectability. Sounds like you're trying to make the list deletable after each append.

sourcebits-robertbiggs commented 10 years ago

Feel free to paste a code sample here for me to see.

jeff183 commented 10 years ago

when an ajax call is done, the list is emptied and all items are appended. Then the init deletables is done (ie. once only). The weird thing is if the list has one item, no problem. list has 2 items i get the error. And then for 3+ items, no problem. Just the 2 item list gets the error. will paste some code

sourcebits-robertbiggs commented 10 years ago

Could I see a code snippet of how you're doing this?

sourcebits-robertbiggs commented 10 years ago

Here's how I see this happening:

var populateList = function(data callback) {
  data.forEach(function(ctx) {
    $('#list1').append('<li><h3>' + ctx + '</h3></li>');
  });
  if ($.isFunction(callback) {
    callback();
  }
};
$.getJSON('http://boring.com/data.json', function() {
  populateList(function(data, function() {
    $.UIDeletable({
      list: '#list1', 
      callback: function(item) {
        var text = $(item).siblings('h3').text();
        $('#response').html('You deleted: <strong>' + text + '</strong>');
      }
    });
  });
});
jeff183 commented 10 years ago

actually ajax not needed to reproduce the error. Try this, you will get the error on the 2nd load of the list.

var list = $('#list1'); var myArray = ["1","2"];

    $.each(myArray, function(index,ctx) {
        list.append('<li><h3>' + ctx +'</h3></li>');
    });
    initDeletables();

    list.empty();
    myArray.push("3");
    $.each(myArray, function(index,ctx) {
        list.append('<li><h3>' + ctx +'</h3></li>');
    });
    initDeletables();

    list.empty();
    myArray.push("4");
    $.each(myArray, function(index,ctx) {
        list.append('<li><h3>' + ctx +'</h3></li>');
    });
    initDeletables();

    function initDeletables() {
        $.UIDeletable({
            list: '#list1',
            callback: function(item) {
                var text = $(item).siblings('h3').text();
                //$('#response').html('You deleted: <strong>' + text + '</strong>');
                console.log('deleted');

            }
        });
    }
sourcebits-robertbiggs commented 10 years ago

Oh, I see, the problem is not the list, its the Edit button that's already been injected into the header. Multiple execution of $.UIDeletable are causing problems initializing the button when it's already there. Give me a day to work around this. I need to do some thorough testing for edge cases to see what's going on before I push the fix.

sourcebits-robertbiggs commented 10 years ago

OK, so after many hours of debugging, I found out what was happening. First of all, after additional initializations the Edit button was not being found, secondly, the list was getting another delegated event for the delete indicators at each initialization, resulting in the same thing being attempted multiple times with each deletion, depending on how many times the list got initialized. The fix is easy. Open up chui-3.5.5.js.

At line 1445 - 1446 you need to change it from this:

          button = list.closest('article').prev().find('.edit');
        }

To this:

          button = list.closest('article').prev().find('.edit');
        } else {
          button = list.closest('article').prev().find('.edit');
        }

Then, down around line 1478, you'll find this line of code:

$(list).on('singletap', '.deletion-indicator', function() {

You need to add a line to remove any existing delegated events:

$(list).off('singletap', '.deletion-indicator');
$(list).on('singletap', '.deletion-indicator', function() {

Oh yeah, one last fix, around line 1562 look for (causes problem when only one list item):

var height = $('li').eq(1)[0].clientHeight;

Change the .eq(1) to .eq(0):

var height = $('li').eq(0)[0].clientHeight;

I'll be pushing these fixes into the main branch today (version 3.6.1).

jeff183 commented 10 years ago

hey great. much appreciated.

sourcebits-robertbiggs commented 10 years ago

So, because the Edit button was already there on a second init, it wouldn't find it. Then if after multiple inits and it did, the delete indicators would have so many delegated events that the checks would fail.

jeff183 commented 10 years ago

i put the $.UIDeletable inside a try catch, and just ignored the error that i got on the second init. it seemed to work ok.

sourcebits-robertbiggs commented 10 years ago

Jeff, you really should at least implement the line that gets rid of the existing delegates, otherwise you may find you wind up with serious memory leaks and unexpected behavior when multiple versions of the delegates are executing.

$(list).off('singletap', '.deletion-indicator'); $(list).on('singletap', '.deletion-indicator', function() {

Robert Biggs Product Director ChocolateChip-UI www.sourcebits.com

On Jun 27, 2014, at 9:17 AM, jeff183 notifications@github.com wrote:

i put the $.UIDeletable inside a try catch, and just ignored the error that i got on the second init. it seemed to work ok.

— Reply to this email directly or view it on GitHub.

jeff183 commented 10 years ago

yes, i will implement all the changes. thanks Robert