Sort by column keys #181
Sort by column keys #181
Conversation
There was a problem hiding this comment.
👋 Thanks for addressing #48! This looks really solid.
Tests are failing on Travis because https://www.npmjs.com/package/phantomjs-prebuilt is long dead (+ the test framework is a little dusty here in general). Tests pass locally though, so I'm not too worried about that. I just left a couple of small comments then this looks good to go!
This PR takes a more configuration-heavy approach
💯
| </table> | ||
| {% endhighlight %} | ||
|
|
||
| <table id='column-keys' class='sort'> |
There was a problem hiding this comment.
I know the documentation of this project is a little horrendous but for your code example to work, can you add
new Tablesort(document.getElementById('column-keys'));Within the <script> tag here?
There was a problem hiding this comment.
Ah, I incorrectly assumed that the script at the bottom would initiate Tablesorts on all table.sort elements in here. My mistake!
I've given your suggestion a whirl!
|
Thanks for the review, @tristen! I figured the Travis failures were unrelated, good to know! I've addressed your feedback, let me know if there's anything else, thanks! |
|
Published to npm as |
Issue
This PR allows users to fix the issues they encountered in #48.
Context
As noted in #48, tablesort currently assumes that the columns in table headers match the columns in the table body of every row. This leads to arguably erratic behavior when tablesort is applied to tables that utilize colspans.
I believe #130 attempts to rectify this problem by calculating the appropriate column to sort by, accounting for varying colspans in every row. However, it appears this approach wasn't seen to completion as @tristen noted here. There were also some interesting edge cases to be considered with this mathematical approach. What happens when a row doesn't have any cells lining up with the clicked header because of, say, a full-width cell in that row? Do we treat the row as empty, or do we use the value of the full-width cell in sorting by any of the columns?
This PR's Approach
This PR takes a more configuration-heavy approach, adding support for a
data-sort-column-keydata attribute to both headers and cells. Users can configure exactly which cells in each row they wish to be used for sorting when the corresponding header is clicked. This allows for users to "fix" the behavior of their tablesorts even when using colspans, as they are free to manually assign the appropriate column keys to table cells to sort the table as they see fit.Documentation
I've taken a stab at updating the documentation too, but I'm not 100% sure how to test that I've done that correctly. If you can provide guidance on how to test it out, I'd be happy to fix it up if it's broken.