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

Improved AJAX support #183

Merged
merged 1 commit into from
Dec 18, 2012
Merged

Improved AJAX support #183

merged 1 commit into from
Dec 18, 2012

Conversation

Fervens
Copy link
Contributor

@Fervens Fervens commented Nov 27, 2012

The pager previously had some support for AJAX paging. This patch fixes issues with update events being re-triggered continuously and AJAX error handling being bound too many times. It also adds support for passing filtering parameters with the AJAX request. If you have any issues/suggestions with the patch please let me know.

@Mottie
Copy link
Owner

Mottie commented Nov 27, 2012

Oh man, I didn't see this pull request until I just pushed a fix for the pager. I'll have to look at what you've done.

Thanks for all the work you've done!

@thezoggy
Copy link
Collaborator

to see a compare of his master vs your master (with whitespace changes ignore) view:
https://github.com/dhamma/tablesorter/compare/Mottie:master...master?w=1

@thezoggy
Copy link
Collaborator

note, people really need to stop changing line endings... makes it hard to diff the actual changes.
http://stackoverflow.com/questions/170961/whats-the-best-crlf-handling-strategy-with-git

@Mottie
Copy link
Owner

Mottie commented Nov 28, 2012

Thanks @thezoggy! I'll look into doing the normalization in the next update too :)

@Fervens
Copy link
Contributor Author

Fervens commented Nov 28, 2012

Thanks @thezoggy, I'll be sure to be more careful with the line endings in the future.

Keep up the good work @Mottie.

@Fervens
Copy link
Contributor Author

Fervens commented Nov 28, 2012

It appears that I accidentally included an unrelated change that I was using to hide table filter inputs on hidden columns. This causes some trouble if your table is hidden on page load though, so you'd probably want to avoid including this.

505 - t += '<td>';
506 + t += '<td' + (c.$headers.eq(i).is(':visible') ? '' : ' style="display: none"') + '>';

@Mottie
Copy link
Owner

Mottie commented Nov 28, 2012

@dhamma I hate to ask, but can you resubmit this pull request so I can merge it with the current version. It'll speed things up for me in these days of very limited time. That, and I still need to learn how to cherry pick a pull request 😋

@Fervens
Copy link
Contributor Author

Fervens commented Nov 28, 2012

Alright, it looks like the changes automatically pulled into here, and the line endings look better.

@thezoggy
Copy link
Collaborator

id say rebase and push a new one. otherwise you will have some dirty commit history and git blame will always show you as you changed the files one way then back

@Fervens
Copy link
Contributor Author

Fervens commented Nov 28, 2012

It looks like the blame shows up alright on the site and on my windows box; though, I haven't checked on a linux box. I can rebase if you'd like @Mottie, just let me know.

Bad compare: Fervens/tablesorter@Mottie:master...5422244
Blame of my master: https://github.com/dhamma/tablesorter/blame/master/js/jquery.tablesorter.js

@Mottie
Copy link
Owner

Mottie commented Dec 17, 2012

Hi @dhamma!

Yeah, I was looking over the mess in the history and I agree with @thezoggy about using rebase to clean up the history.

Thanks again for all your hard work!

… re-triggered continuously and AJAX error handling being bound too many times. It also adds support for passing filtering parameters with the AJAX request.
@Fervens
Copy link
Contributor Author

Fervens commented Dec 17, 2012

I was having trouble figuring out how to rebase since I had previously merged, so I ended up just applying the merged changes in a single commit.

@Mottie Mottie merged commit 3d41533 into Mottie:master Dec 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants