Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Events, updateComplete Fires early #343

Closed
TheSin- opened this issue Jun 26, 2013 · 14 comments
Closed

Events, updateComplete Fires early #343

TheSin- opened this issue Jun 26, 2013 · 14 comments
Labels

Comments

@TheSin-
Copy link
Collaborator

TheSin- commented Jun 26, 2013

I seems that updateComplete fires before pager+ajax is done.

This isn't totally wrong, technically, I was hoping for something like

updateComplete->ajaxStart->ajaxEnd

or

updateComplete->ajaxStart->updateComplete

I added an ajaxStart trigger on line 311 of query.tablesorter.pager.js

                                tc.$table.trigger('ajaxStart');
                                $.ajax(c.ajaxObject);

which is working, but it does

ajaxStart->updateComplete (but it's not complete, the table isn't done rendering)

Maybe updateComplete needs to be later? or maybe we need a renderComplete?

@Mottie
Copy link
Owner

Mottie commented Oct 3, 2013

I can do a little reworking, but let me ask this first... if I made updateComplete fire at the same time as pagerChange (line 283), would that work for you? I see this sequence now: ajaxStart -> pagerChange -> updateComplete

if (p.initialized) {
    $t.trigger('pagerChange', p);
    $t.trigger('updateComplete');
}

I also made this change to the core:

function resortComplete($table, callback){
    var c = $table[0].config;
    if (c.pager && !c.pager.ajax) {
        $table.trigger('updateComplete');
    }
    if (typeof callback === "function") {
        callback($table[0]);
    }
}

@TheSin-
Copy link
Collaborator Author

TheSin- commented Oct 3, 2013

I'll have to find that again in my code, see how I fixed it. I just assumed updateComplete was the very last trigger, after addons etc etc, so that I could run row calculations and sure from that trigger.

@Mottie
Copy link
Owner

Mottie commented Oct 3, 2013

It is the last event, when ajax isn't being used 🙀

@TheSin-
Copy link
Collaborator Author

TheSin- commented Oct 3, 2013

do we need an ajaxComplete? I suppose I could just attach tot he jquery ajax event but I think keeping things in the tablesorter config would be better. Maybe in tablesorter ajax call we could add a .onComplete to the ajax call that fires ajaxComplete or something? How does that sound?

@Mottie
Copy link
Owner

Mottie commented Oct 3, 2013

Well, I don't really want to complicate things more... I think I can make it work so that updateComplete is fired when ajax is complete. That way you don't need to bind to different events.

@TheSin-
Copy link
Collaborator Author

TheSin- commented Oct 3, 2013

that works for me, I'm happy to kepe the same trigger, and honestly that makes the most sense. Specially if you have a fix for it.

@Mottie
Copy link
Owner

Mottie commented Oct 3, 2013

Cool, but is it working for you? It seems to be working properly for me, but I haven't extensively tested it yet.

@TheSin-
Copy link
Collaborator Author

TheSin- commented Oct 3, 2013

still trying to find the example that was not working for me so I can test ;) My project is very large and I use tablesorter extensively throughout it ;) As soon as I find the page I'll report back.

@Mottie
Copy link
Owner

Mottie commented Oct 8, 2013

Hey @TheSin-!

Any news on this? I'm getting closer to finishing up the next update and hoping that this fix worked. :P

@TheSin-
Copy link
Collaborator Author

TheSin- commented Oct 9, 2013

sorry I'm not forgetting or ignoring just been sooooo busy as of late. I'll seriously try and test this tomorrow to be sure, but I do believe just looking at the code that it will work.

@Mottie Mottie closed this as completed in 29c5bf4 Oct 11, 2013
@Mottie
Copy link
Owner

Mottie commented Oct 11, 2013

crosses fingers

@TheSin-
Copy link
Collaborator Author

TheSin- commented Oct 11, 2013

I'm feeling good about it, and you are a beast today!!! I can't wait to test to new release!!

@Mottie
Copy link
Owner

Mottie commented Oct 11, 2013

And all without a caffeine boost! ;P

@TheSin-
Copy link
Collaborator Author

TheSin- commented Oct 11, 2013

damn it man someone get this man a caffeine drip stat!!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants