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

Tbodysort widget enhancement. #1312

Merged
merged 5 commits into from
Nov 17, 2016
Merged

Tbodysort widget enhancement. #1312

merged 5 commits into from
Nov 17, 2016

Conversation

PurplProto
Copy link
Contributor

I needed to sort my <tbody>'s and their child rows, but I wanted to keep the header rows at the top of the parent <tbody>, so I added a little extra code to accomplish this since I didn't know of any other widget that could do it.

I haven't been able to unit test my code since I have a tight schedule at the moment, however its a very small snippet and not much could go wrong. It's working and I'm using it on my production server too, so should any issue arise, I'll be back to fix it.

Short description of what this new widget option sortTbody_lockHead does:
After the table has finished sorting, the table rows with the same class tag value specified in cssHeader inside the config js/jquery.tablesorter.js is moved to the top of it's parent <tbody> .

@Mottie
Copy link
Owner

Mottie commented Nov 8, 2016

Hi @ChrisM-Rogers!

Thanks for sharing your work. Would you please provide a demo of how this modification is expected to behave? I created this demo and I am not seeing any difference.

@PurplProto
Copy link
Contributor Author

Sorry, I should have been a little more specific! This is my first contribution to an opensource project, but I hope to improve :)

I think my commit explains it better really, but it's intended to be used with the sortTbody_sortRows option set to true since I wish to sort the child rows of the <tbody> but I don't want the header row to be included in that sort, I want it to stay as a header.

So as well as the sortTbody_sortRows : true, you'd need sortTbody_lockHead : true and then have your header rows in the <tbody> to have the same class name specified in the cssHeader setting in the main JS file js/jquery.tablesorter.js. By default, that has an empty string '', so you'd need to set that for it to work.

I've just found that I must have made a mistake in the code I've committed too, I'll fix that asap, and I haven't pulled my enhancement branch into my master yet either, (oops) I'll have that sorted too asap. I'll let you know when its done.

Last of all, I'll get a JSFiddle example too :)

@PurplProto
Copy link
Contributor Author

I've just seen where i can improve this a bit. Instead of using the cssHeader value to find the the header row, I should use the sortTbody_primaryRow value instead!

A quick commit later and I'll be back with a JSFiddle example.

@PurplProto
Copy link
Contributor Author

Hum, it seems there's some differences between my work and the demo... I'll have a look further when I've got a bit more time. Not sure what the issue is but as soon as I link the widget with my additions from Github (using the raw data), it breaks the demo.

@PurplProto
Copy link
Contributor Author

I found that Github doesn't like direct linking, but I've found another way to get it linked 👍 Now its just the just an issue with the code. I've not given up hope yet!

@Mottie
Copy link
Owner

Mottie commented Nov 10, 2016

You can use http://rawgit.com/ to link to github files. But I would not recommend using the "URL in production" link that page creates as it is links to a file permanently. It will not update if the file changes.

@PurplProto
Copy link
Contributor Author

I finally have a working Fiddle example!

This is how I intended it to work, its using the files from my master branch. So what happens is, the sort works as it should, but because the .main row is included in the sort, I wanted a way to keep it static basically. So after all rows are sorted, the .main row is moved back to the top of its <tbody>.

var lockHead = wo.sortTbody_lockHead;
var primaryRow = wo.sortTbody_primaryRow;

if ( primaryRow ) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be changed to if ( lockHead && primaryRow ) {

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, since lockHead is only used once... change that line to:

if ( wo.sortTbody_lockHead && primaryRow ) {

and remove the var lockHead = wo.sortTbody_lockHead;

@Mottie
Copy link
Owner

Mottie commented Nov 16, 2016

By the way, you can accomplish the same thing by moving the "headers" into a separate tbody with a "tablesorter-no-sort-tbody" class (demo); but I don't mind merging this PR to simplify the method.

<table>
  <thead><!-- ... --></thead>

  <tbody class="tablesorter-no-sort-tbody">
    <tr><!-- fixed header row --></tr>
  </tbody>
  <tbody>
    <tr class="main"><!-- sortable tbody --></tr>
  </tbody>

  <tbody class="tablesorter-no-sort-tbody">
    <tr><!-- fixed header row --></tr>
  </tbody>
  <tbody>
    <tr class="main"><!-- sortable tbody --></tr>
  </tbody>

</table>

Added two new variables to hold the lockhead option and the head row class (defined in cssHeader).
If the new widget option 'sortTbody_lockHead' is true, the header row will be moved back to the top of the <tbody> after sorting, this is intended to be used with the 'sortTbody_sortRows' option set to true.
This will allow the <tbody> and it's child rows to be sorted while keeping the header row at the top of the <tbody>.
…tead of the `cssHeader` value.

This means the new widget option can be used out-of-the-box, meaning the user won't need to change any default parameters in the js files.
  - Removed `lockHead` var since it's only used once
  - Ammended `sortEnd` condition
@PurplProto
Copy link
Contributor Author

PurplProto commented Nov 17, 2016

Ah I see. I was in a bit of a rush when I needed this function, and when I came across the method you've mentioned, it looked like it was sorting all the child rows together and a child row might end up moving under a different parent header (or main) row. I was a little worried that might happen.

I've done the changes and rebased to your master.

EDIT:
Checked the Fiddle you mentioned and it doesn't completely replicate what I needed, the header rows are completely static and don't move, where as in my Fiddle, the whole tbody is sorted as well as its contents so the header rows will appear sorted i.e. it starts in the order of 'Dealer 48', 'Dealer 29' then 'Dealer 62', I wanted these to sort as well so it ends up being 'Dealer 29', 'Dealer 48' and then 'Dealer 62' as well as the child rows being sorted too.

@Mottie
Copy link
Owner

Mottie commented Nov 17, 2016

Ok, looks good! Thanks!

@Mottie Mottie merged commit 43979d4 into Mottie:master Nov 17, 2016
Mottie added a commit that referenced this pull request Nov 18, 2016
@PurplProto
Copy link
Contributor Author

Awesome! And thank you for maintaining it! 😄

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

Successfully merging this pull request may close these issues.

3 participants