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

pager plugin with child rows #396

Closed
ghost opened this issue Oct 25, 2013 · 34 comments
Closed

pager plugin with child rows #396

ghost opened this issue Oct 25, 2013 · 34 comments

Comments

@ghost
Copy link

ghost commented Oct 25, 2013

The pager plugin does not take account of child rows, basing it's calculation for the number of pages and how many rows to put on each page by the rows array. Since child rows are added to their parent, they get missed in these calculations.

This is easily seen by adding class="tablesorter-childRow" to some of the rows in http://jsfiddle.net/Mottie/4mVfu/1/. For example, if I add the class only to the <tr> for Student02, it shows 11 students on the first page, with a page size of 10.

@Mottie
Copy link
Owner

Mottie commented Oct 25, 2013

Hi @graememclean!

Hmm, well I guess I was assuming that the child rows were hidden and could be opened by a button/link in the parent row. I guess I'll need to include an option to include or not include child rows in the count... I wish I could just detect if the row is visible but if you look at the child row demo, it is actually hiding a table cell in the child row(s) and not the row itself.

@ghost
Copy link
Author

ghost commented Oct 25, 2013

My use case is an item with sub-items, each of which can have sub-items etc. I use the child row functionality to make the sub-items stay with the item when the items are sorted. What I would really like would be multiple levels of children and the ability to sort the children within the parent. But maybe I'm asking too much... ;-)

@Mottie
Copy link
Owner

Mottie commented Oct 25, 2013

Can you share some example HTML? Or better yet update that demo with the HTML 🍮

@ghost
Copy link
Author

ghost commented Oct 26, 2013

I've created this fiddle http://jsfiddle.net/graememclean/2bcYN/ to try and show what I mean. I'm using the child row functionality to ensure sub-items and sub-sub-items always stay with the parent when the parents are sorted. However I would really like to be able to sort sub-items within their item at the same time (e.g. sort by category and items seven and eight should swap places within item six). This example also shows the issue I'm having with the page count as it only counts the main items, ignoring the children.

@Mottie
Copy link
Owner

Mottie commented Oct 26, 2013

Wow, so you want items 4 & 5 to only sort within item 3, and items 12 - 14 to only sort within 11?

I think this I can get it to work with external coding, but not necessarily require me to rewrite the plugin to accommodate.

Give me some time.

@Mottie
Copy link
Owner

Mottie commented Oct 30, 2013

I just added a new pager option countChildRows (Boolean) which forces the pager to count child rows. I still haven't had time to work on the child row sorting demo for you. I'll try to get to it this weekend.

@ghost
Copy link
Author

ghost commented Oct 31, 2013

This change works perfectly. Thank you very much for doing it so quickly.

@ghost
Copy link
Author

ghost commented Nov 1, 2013

Hi,

Having now introduced filters I'm hitting a different problem with the page count.

Since the paging calculations are using a min(totalPages, filteredPages) method, paging is not working correctly when nothing is filtered because totalPages is not taking into account the child rows.

I'm wondering if the calculation of c.totalRows in the appender function of tablesorter needs to take into account child rows instead of simply taking rows.length?

Thanks, graeme.

@Mottie
Copy link
Owner

Mottie commented Nov 1, 2013

Hmm, well I just tested this change to the appender code:

$this.appender = function(table, rows) {
    var c = table.config,
        p = c.pager;
    if ( !p.ajax ) {
        c.rowsCopy = rows;
        p.totalRows = p.countChildRows ? c.$tbodies.eq(0).children().length : rows.length;

And it works; but after testing it on the demo you shared, I realize that the category column won't filter correctly because all child row data is merged in with it's parent row data... I guess I'll have to make some changes to the filter widget as well.

The main reason why child rows were treated this way is that I assumed (bad call I know) that all child rows would have a rowspan and/or colspan shared with the parent so it would be difficult to determine which column of data to filter. I could use the same logic code that determines the correct column in the header and it would work, but it would be very inefficient to make this determination for every row in the table as it would slow down initialization drastically (in large tables). So I'll try to apply this logic code on just parent/child rows and see what I can do with the filter widget then...

Edit: Oops,code correction above.

@ghost
Copy link
Author

ghost commented Nov 1, 2013

I made a similar change to the appender function and everything is fine except that child rows not included in the filter were still visible after paging took place. The filter widget was correctly putting a 'none' style display on the tr rows but paging is looking for the 'filtered' class.

I can get around this by forcing the filteredRow class onto the child rows near the foot of the findRows function in the filter widget:

...
$tr.eq(j)[r ? 'removeClass' : 'addClass'](wo.filter_filteredRow);
if (cr.length) { 
    cr[r ? 'removeClass' : 'addClass'](wo.filter_filteredRow);
    cr[r ? 'show' : 'hide']();
}

The problem being the row i added to stick the 'filtered' class on should be dependent on countChildRows which is a pager parameter, nothing to do with filtering. I don't know if this parameter is visible at this point.

@Mottie
Copy link
Owner

Mottie commented Nov 2, 2013

Hmm, well the filter widget would have access to the pager options... pager options are stored in table.config.pager (and in table.config.widgetOptions for the pager widget)

So, you should be able to change that line to:

$tr.eq(j)[r ? 'removeClass' : 'addClass'](wo.filter_filteredRow);
if (cr.length && (c.pager && c.pager.countChildRows || wo.pager_countChildRows)) {
    cr[r ? 'removeClass' : 'addClass'](wo.filter_filteredRow);
    cr[r ? 'show' : 'hide']();
}

If that works, I'll include it in the next update. Thanks for all your help with the troubleshooting and coding! :)

@ghost
Copy link
Author

ghost commented Nov 2, 2013

Fantastic. Yes that change works. I actually coded it as:

$tr.eq(j)[r ? 'removeClass' : 'addClass'](wo.filter_filteredRow);
if (cr.length) {
    if (c.pager && c.pager.countChildRows || wo.pager_countChildRows) {
        cr[r ? 'removeClass' : 'addClass'](wo.filter_filteredRow);
    }
    cr[r ? 'show' : 'hide']();
}

Since cr[r ? 'show' : 'hide'](); is part of the original functionality, I guess it should still happen even if childCountRows is false.

Thanks again for your help and your really quick responses.

@Mottie
Copy link
Owner

Mottie commented Nov 2, 2013

Oops, you're right, the show/hide should be outside of that block... need caffeine! LOL

I'll include this in the next update :)

@Mottie Mottie closed this as completed in 4ea59d8 Nov 2, 2013
@Mottie Mottie reopened this Nov 3, 2013
@Craga89
Copy link
Contributor

Craga89 commented Nov 4, 2013

Hey guys, just to jump on this issue, I presume this also causes the problem with child-rows of the previous page being included in the header of the next?

Demo of this can be seen on the docs page (you can't open up the last child row, since it's included on the page after): http://mottie.github.io/tablesorter/docs/example-child-rows.html

@Mottie
Copy link
Owner

Mottie commented Nov 4, 2013

Hi @Craga89! Yeah, that is the problem I noticed too. I'm not sure how to solve it.

And FYI, @steenreem is working on hierarchical sorting in his fork which should solve the other issue here of sorting the child rows.

@ghost
Copy link
Author

ghost commented Nov 4, 2013

Hi @Mottie, I've found another couple of problems with using filters and child rows together.

Firstly the filter_childRows will cause a match if the filter text is anywhere in the child row. For my item, sub-item scenario this is confusing to the user. It would be nice if there was another option to restrict to match only within the same column as the filter. I've made this work by changing the definition of t in findRows which gets added to xi for child row searching. Instead of just taking cr.text(), I loop through cr.children[i].textContent to build up the value of t.

The second issue is with select filters. I use just the default select but in buildSelect it only pushes the cell for c.cache[k].row[j][0] which means if the select value is only being used by the child row it's missing from the select options. I've put another loop inside the j loop to go around 0 to c.cache[k].row[j].length and add on the child value to arry.

The third thing, also for select filters, is I had to extend slightly the way findRows matches the filter value to do similar with text filters and include the child values: ff = ... ? (xi + t).search(val) >= 0 : (x+tRaw).search(v[i]) >= 0; where tRaw is the value of t before appling toLocaleLowerCase so that I match on the value in the dropdown.

I do realise I'm using child rows in a different way from how they were originally designed... ;-)

@keyboardDrummer
Copy link

Hi @graememclean,

I'm working on exactly the feature that you need: sorting within a fixed hierarchy. You can define a hierarchy for the table using classes on the rows. Sorting is done first on the root nodes, then for each root node on its children, then for each child on its children, etc..
I don't see how you could use childRows to accomplish the same, because they only support 1 level deep hierarchies and no sorting of the childRows, as you mentioned.

If you're interested, have a look at the jquery.tablesorter.js in my fork. There's still a bunch of testing work to be done, particularly in combination with the widgets.

The classes used to specify the hierarchy are currently non-customizable and are the same classes as used by the 'treeTable' plugin: "data-tt-id" and "data-tt-parent-id".

@Craga89
Copy link
Contributor

Craga89 commented Nov 5, 2013

@Mottie One interesting thing to note is that the original demo you link to on the docs page doesn't exhibit this behaviour. Perhaps the solution is in that code?

@Mottie
Copy link
Owner

Mottie commented Nov 5, 2013

@Craga89 That original demo is doing essentially the same thing as having the countChildRows set to false. Also, the parent row shares a column with it's child rows via a rowspan so if we did set the countChildRows option to true, it would break the table layout when a child row is shown without the parent.

@keyboardDrummer
Copy link

Here's a small example of the hierarchical / tree sorting that I'm working on: http://steenreem.github.io/tablesorter/docs/example-tree-table.html

@keyboardDrummer
Copy link

@graememclean
How do you want the pager plugin to behave in combination with a tree structured table?

I imagine that you wouldn't want part of a tree being cut off on one page and then continuing on the next page, like this:

Page1:
Parent1
Child1
Child2

Page2:
Child3

One proper behavior I can imagine is the following. The pager plugin only takes into account the root nodes to count the size, and always puts complete root nodes on one page. This results in pages having an equal amount of root nodes. The total number of rows in each page can differ however, when the root nodes have a different number of children.

@Fuzzyma
Copy link

Fuzzyma commented Nov 14, 2013

I run into similar issues.
My script seems to ignore the countChildRows option since there are always less then the expected parent-nodes. The wired thing is that even with the childrowes counted its not the right count.
One time more one time less. I have no idea what this pager is counting in any case.
I will try to set up a fiddle which shows this problem.

//EDIT: Here is the fiddle which shows the problem:
http://jsfiddle.net/9UW6k/1/
Note that there are multiple childs which are divided into one and twolevel chields but nothing which should break the system

@Mottie
Copy link
Owner

Mottie commented Nov 14, 2013

Hi @Fuzzyma!

The current default class name for cssChildRows is tablesorter-childRow. So if you set it to match your expand-child class name, the demo only finds 20 rows (updated demo):

    $(".tablesorter:not(.tabledata table)").tablesorter({
        cssChildRow: 'expand-child'
    })

@Fuzzyma
Copy link

Fuzzyma commented Nov 14, 2013

Wow - how could I miss that? I looked at the example and there was written
that tablesorter-childrow is only for this example and that the standard
would be expand-child. Thank you for that tip!
Am 14.11.2013 18:17 schrieb "Rob G" [email protected]:

Hi @Fuzzyma https://github.com/Fuzzyma!

The current default class name for cssChildRows is tablesorter-childRow.
So if you set it to match your expand-child class name, the demo only
finds 20 rows (updated demo http://jsfiddle.net/Mottie/9UW6k/2/):

$(".tablesorter:not(.tabledata table)").tablesorter({
    cssChildRow: 'expand-child'
})


Reply to this email directly or view it on GitHubhttps://github.com//issues/396#issuecomment-28503171
.

@ar-lab
Copy link

ar-lab commented Feb 20, 2014

Hi!
For me is also need a option in filter widget to filtering from all data in children rows or data from current column only.

My modification of filter widget for filtering child rows only in current column:
http://jsfiddle.net/Mottie/3359Y/1/

@Mottie
Copy link
Owner

Mottie commented Feb 20, 2014

Hi @ar-lab!

Thanks for sharing! I'll look into this.

Mottie added a commit that referenced this issue Mar 31, 2014
Add tablesorter-hasChildRow
If the last pager row has any child rows, they are now included
Add "filtered" css definition to every theme to hide content
@Mottie
Copy link
Owner

Mottie commented Mar 31, 2014

@Craga89 Sorry for taking so long, but the issue with the pager last row not including child rows has been fixed.

@ar-lab I finally got a chance to look at your code. There are a lot of differences in the code, but I managed to see the changes. I'll need to work on it a bit more as that code does not take into account colspans. I think I might just add an option to allow choosing between searching all child row content versus child row columns.

@mecsco
Copy link

mecsco commented Apr 1, 2014

Absolutely tremendous timing @Mottie - I just encountered this issue, came across this thread and found that you fixed it yesterday - thank you very much

@Craga89
Copy link
Contributor

Craga89 commented Apr 1, 2014

@Mottie awesome! Thanks for taking the time to look at this 👍

@Mottie
Copy link
Owner

Mottie commented May 15, 2015

@ar-lab Ok, I finally got around to making the filtering of child row content by column to work. Please see this comment for more details.

@Craga89
Copy link
Contributor

Craga89 commented May 16, 2015

👍 to getting back to this. I know all too well how easy it is to let this stuff slip

looks at own repos massive backlog

@Mottie
Copy link
Owner

Mottie commented May 16, 2015

@Craga89 Check out ZenHub, it's really been helping me organize and prioritize issues ⚡

@Craga89
Copy link
Contributor

Craga89 commented May 16, 2015

+1 to that @Mottie, looks awesome

@ar-lab
Copy link

ar-lab commented Jul 3, 2015

@Mottie Good job!

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

No branches or pull requests

6 participants