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

Usage of tablesorter-headerSortDown, tablesorter-headerSortUp aren't matching documentation #173

Closed
bitti opened this issue Nov 10, 2012 · 5 comments
Labels
Milestone

Comments

@bitti
Copy link

bitti commented Nov 10, 2012

Please have a look at

http://mottie.github.com/tablesorter/docs/#Demo and
http://mottie.github.com/tablesorter/docs/#Configuration
https://github.com/Mottie/tablesorter/blob/master/js/jquery.tablesorter.js#L74

As you can see in the demo the tablesorter-headerSortUp class is used for columns wich are sorted descending and the tablesorter-headerSortDown is used when the sort order is ascending. This is a direct contradiction to the documentation in the Configuration section and in the source code.

To add confusion the arrow attached to tablesorter-headerSortUp is a down arrow and the other way around resp. Therefore the arrows are matching the sort direction even though the classes don't.

@Mottie
Copy link
Owner

Mottie commented Nov 10, 2012

Yeah, I got confused a few times looking at that too. I've tried to keep it the way tablesorter was set up originally, but maybe it got turned around somewhere.

Honestly, I think it might be better to just rename the default classes to tablesorter-headerAsc and tablesorter-headerDesc. Which I might just do in the next update.

@bitti
Copy link
Author

bitti commented Nov 10, 2012

I don't think you turned it around somewhere, the bug is even present in the original TableSorter from Christian Bach. Anyway I also think Asc end Desc would be much better suffixes since these are self explanatory.

@Mottie
Copy link
Owner

Mottie commented Nov 14, 2012

Well I don't want to make anymore major changes to tablesorter until I finish version 3, which already includes this change. So I'm going to mark this issue for version 3.

Thanks for the input! :)

@Mottie
Copy link
Owner

Mottie commented Nov 24, 2012

This issue should now be fixed. I added the new tablesorter-headerAsc and tablesorter-headerDesc classes to the existing list to maintain backwards compatibility. But a bunch of the images have swapped and the uitheme widget has reversed which icons are used, so it's always best to use the latest version. The older class names in the css will be removed in version 3.

@Mottie Mottie closed this as completed Nov 24, 2012
@bitti
Copy link
Author

bitti commented Nov 26, 2012

Thank you for taking this so seriously! I think it's much clearer now.

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