Mottie / tablesorter

Github fork of Christian Bach's tablesorter plugin + awesomeness ~
https://mottie.github.io/tablesorter/docs/
2.61k stars 754 forks source link

filter_formatter + stickyHeaders #290

Closed Mottie closed 11 years ago

Mottie commented 11 years ago

filter_formatter elements do not work when combined with the stickyHeaders widget.

This is because the sticky header widget duplicates the elements in the header but not the scripting. It's a bit more complicated than that, because simply duplicating the scripting doesn't make it work properly. I'm adding this issue to keep track of this problem until I have time to work out a clean solution.

TheSin- commented 11 years ago

is there any way to disable filters only on the sticky header this this is resolved? my users will be very confused by this issue

EDIT: I found this to hide them for now at least

initialized: function(table) {
        $('table.containsStickyHeaders tr.tablesorter-filter-row').hide();
},
TheSin- commented 11 years ago

Also as a suggestion could we rename the class or add a class to the sticky header filters

tablesorter-filter -> tablesorter-sticky-filter or tablesorter-filter stickyheader

Then in the code we could run it on both sets and updating both sets to match, etc etc

just a suggestion, I'm sure you know more about it then me but I'm trying to figure out how to help so reading through it slowly.

Mottie commented 11 years ago

Hi @TheSin-!

Actually, this isn't a duplicate of issue 288, because this one involves the additional filter_formatter functions, but in the same row... it's a bit more complicated than the filter-select fix, so I opened up a separate issue.

You can also hide any of the sticky elements by using css to target the sticky table which contains the thead, caption and/or filter row:

/* target the table */
table.containsStickyHeaders .tablesorter-filter-row { display: none; }

/* target the thead */
thead.tablesorter-stickyHeader .tablesorter-filter-row { display: none; }

I don't really want to rename the sticky header filter class names because that would require adding addition css to every theme.

TheSin- commented 11 years ago

ahh, sorry I assumed it would be from the same cause, sorry about that.

And I'm not sure why I didn't just use css too busy with trying to make the headers match, mine are very bad currently and since it's a private site I can't post a link but basically with the why I'm using them it's not working at all with the new stickyHeaders, I'm trying to figure out if it's my css or because of my colSelector plugin I wrote.

For now I'll add that CSS though that will save me a ton of time so that I can get this update into production.

and as for the rename I just thought it's allow use to update both inputs a little easier and since we'd know which one is being changed we could update the other one a little easier.

Mottie commented 11 years ago

No worries! It's not easy getting all of the widgets to cooperate with each other. Sometimes I forget to include support for one, or two ;)

TheSin- commented 11 years ago

well honestly you are doing an amazing job I'm just trying to help where I can. Let me know if you need me to make any local changes to test anything at any point I have lots of different usages of tablesorter so I can test lots of different combinations at once ;)

TheSin- commented 11 years ago

I'm thinking about working on this one today, is there any info you have Mottie before I start so we don't duplicate efforts?

Mottie commented 11 years ago

I haven't started working on the jquery.tablesorter.widgets-filter-formatter.js file yet, as that is where most of the changes will need to be made.

I haven't had much of a chance to think about it, but since the stickyHeaders widget is initialized after the filter widget, every block of code within the filter formatter file will have to need some way to detect if table.config.widgetOptions.$sticky exists after the table has initialized (maybe inside the "update" function would be enough?), then get into the sticky header and replace, modify or update the custom controls there.

TheSin- commented 11 years ago

sweet nice work Rob thanks from all of us! :D

ymTm7KuhCnIjkZAl2x2m2 commented 11 years ago

This still doesn't seem to work with uiDatepicker. Tested http://mottie.github.io/tablesorter/docs/example-widget-filter-formatter-1.html on Chrome, FireFox, and IE, the rightmost filter disappears on scrolling.

TheSin- commented 11 years ago

I think it's just an issue with the only version not being updated, I use datepicker with the big 4 in a production env and I have no issues with it with stickyheaders.

ymTm7KuhCnIjkZAl2x2m2 commented 11 years ago

I don't understand. Are you saying there's a workaround? I think I'm using the latest code and seeing the same issue as that demo.

TheSin- commented 11 years ago

I'm not 100% sure maybe try a git checkout version maybe the fix isn't released yet? All I'm saying is that it is working for me, I know I have some changes to mine that aren't on git yet but I'm pretty sure that one is.

ymTm7KuhCnIjkZAl2x2m2 commented 11 years ago

I can only get it to work if I comment out this line. It looks like it overwrites t, which is THEN appended to the .tablesorter-filter-row.

t = o.from ? ( o.to ? o.from + ' - ' + o.to : '>=' + o.from ) : (o.to ? '<=' + o.to : '');

TheSin- commented 11 years ago

is this from git? if so what line of which file so I can look at it closer in context?

ymTm7KuhCnIjkZAl2x2m2 commented 11 years ago

https://github.com/Mottie/tablesorter/blob/master/js/jquery.tablesorter.widgets-filter-formatter.js line 580. It looks like it's messing up the context for t on line 565. In my code I changed 580 to a local variable, and changed 581 to use that local variable. My temporary workaround at least.

TheSin- commented 11 years ago

ahh yes I see it now really there doesn't even need to be var there at all, could just return it. Though it is odd that over writing the var at the point is causing an issue, but if you say it fixes it for you lets just change out that var.

                tret = o.from ? ( o.to ? o.from + ' - ' + o.to : '>=' + o.from ) : (o.to ? '<=' + o.to : '');
                return $input.val( tret );

does this work for you then? If so I'll commit it.

ymTm7KuhCnIjkZAl2x2m2 commented 11 years ago

Well var tret = ... but yes, that fixes the stickyheader issue. Though when I scroll down and use that date filter on the sticky header, the functionality is wonky (the filter doesn't seem to apply until I select it twice). But that could just be me.

TheSin- commented 11 years ago

okay I'll spend more time one this and figure it out properly not that i have it failing on my side properly, I'm just tryin got remember how to do a clean checkout with write access, I think maybe Mottie removed my commit privs if so i can't fix this for you, but it could just be me, it's been a long week so I'm having a bit of github issues atm.

TheSin- commented 11 years ago

Okay i have it 100% fixed now, but I need ot figure out how to commit

fixed in commit 1d82261a2c6c4ff947d36e8bbff399fae97fd2a1

thank you for the report, and sorry it took so long to fix, I had fixed the single date variant, I didn't use the dateCompare so that is my fault.