Bug 37148

Created attachment 168175 [details] [review]
Bug 37148: Moved filter bar to the top of Holds to Pull report table.

To Test:

1. Go to Circulation -> Holds to pull
2. Notice filter bar is at the bottom of the table.
3. Apply patch, restart_all
4. See filter bar at the top of the table.

Instead of putting the filters into the same <thead>, I recommend just doing a new <thead></thead> underneath the existing one and then putting the filter code in there. 

This makes everything less crowded and with the current patch, it's hard to see what you type in for some of the filters, specifically "Pull this many items".

Looks like the patch breaks the Columns drop down menu of the Hold to pull report. After applying the patch to a sandbox, Columns menu only has the little check marks without the column names. I tried with two PTFS Europe's sandboxes.

We're going to resurrect this one

Created attachment 170759 [details] [review]
Bug 37148 - Move hold to pull column filters to top

This bug moves the column filters on the holds to pull page (pendingreserves.pl)
to the table header, similar to how it is being done on shelves.pl. This
should make it much clearer to users that filtering is possible, and also
make the filters float on the page, so they are always visible.

To test:
a)  apply half a dozen holds, with varying item types, shelving locations,
    holding branches, titles, and collections
b)  go to pendingreserves.pl
c)  notice that the filters are in the table's footer, and that you have
    to scroll down to see them
d)  APPLY PATCH
e)  notice that on pendingreserves.pl, the filters present in tfoot are
    now in thead
f)  notice that the filters are unchanged, aside from location on page
g)  notice that the filters now stick to the top of the page, like the
    headings did
h)  notice that the headings still change the sort order when clicked
i)  notice that the filter inputs all still work as expected
j)  SIGN OFF! :-)

Could we consider doing this to all tables with the filter options? 
I think they never really make sense at the bottom: when I have so many results I want to filter, I don't want to have to scroll down first.

Agreed; it seems silly to have them at the bottom. Let's get this signed-off, and then I can work on a separate omnibus bug for the rest of them?

Jake.

Do we need to obsolete the first patch here now Jake?

(In reply to Martin Renvoize from comment #9)
> Do we need to obsolete the first patch here now Jake?

Answer: Yes.. the new patch is a fresh patch.

Created attachment 170955 [details] [review]
Bug 37148 - Move hold to pull column filters to top

This bug moves the column filters on the holds to pull page (pendingreserves.pl)
to the table header, similar to how it is being done on shelves.pl. This
should make it much clearer to users that filtering is possible, and also
make the filters float on the page, so they are always visible.

To test:
a)  apply half a dozen holds, with varying item types, shelving locations,
    holding branches, titles, and collections
b)  go to pendingreserves.pl
c)  notice that the filters are in the table's footer, and that you have
    to scroll down to see them
d)  APPLY PATCH
e)  notice that on pendingreserves.pl, the filters present in tfoot are
    now in thead
f)  notice that the filters are unchanged, aside from location on page
g)  notice that the filters now stick to the top of the page, like the
    headings did
h)  notice that the headings still change the sort order when clicked
i)  notice that the filter inputs all still work as expected
j)  SIGN OFF! :-)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

All works as described.. so signing off.

However, the commit message format is wrong.. remember to run the QA script against your patches Jake.. it'll highlight that sort of minor error (: instead of - please ;P )

I wonder about generalising as Katrin has asked.. though I can't entirely remember how these bits fit together right now.

So.. we actually already have the 'add_filters' option available to us in the KohaTable component.. however for some reason it doesn't work as expected for this table.

I think this patch is OK as is.. but we could investigate why it doesn't work here if we wanted..  or just not worry and leave it to the ajax tables when that eventually rolls around to this particular case?

Ah, nicely spotted — you might need to show me where to find those QA scripts and the best options to use :-)

Created attachment 172279 [details] [review]
Bug 37148: [WIP] use built in table filtering

Created attachment 172280 [details] [review]
Bug 37148 - Move hold to pull column filters to top

This bug moves the column filters on the holds to pull page (pendingreserves.pl)
to the table header, similar to how it is being done on shelves.pl. This
should make it much clearer to users that filtering is possible, and also
make the filters float on the page, so they are always visible.

To test:
a)  apply half a dozen holds, with varying item types, shelving locations,
    holding branches, titles, and collections
b)  go to pendingreserves.pl
c)  notice that the filters are in the table's footer, and that you have
    to scroll down to see them
d)  APPLY PATCH
e)  notice that on pendingreserves.pl, the filters present in tfoot are
    now in thead
f)  notice that the filters are unchanged, aside from location on page
g)  notice that the filters now stick to the top of the page, like the
    headings did
h)  notice that the headings still change the sort order when clicked
i)  notice that the filter inputs all still work as expected
j)  SIGN OFF! :-)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

I got the built in filters to show, but styling was wrong and no branch dropdown - PQA here - having good instructions on how to add filters would be nice

Yeah, it's an odd case.. I'm not at all sure why the built in filters aren't working nicely in this particular table.

I think this is an OK middle ground for now, but I'd love to understand what's going on later.

+    [% IF print %]
+        <script>
+            $( document ).ready(function() {
+                window.print();
+                window.onafterprint = function () {+                    window.close();
+                }
+                setTimeout('window.close()', 1000); // Hack for Chrome < 63
+            });
+        </script>
+    [% END #/print %]

Why do we have this?

(In reply to Nick Clemens (kidclamp) from comment #15)
> Created attachment 172279 [details] [review] [review]
> Bug 37148: [WIP] use built in table filtering

What's wrong exactly with this patch? Why is the styling wrong?

We cannot add the dropdown in the column filters with KohaTable (columns_settings.inc), only using kohaTable (datatables.js, ie. when using the REST API)

Can you please take a look at Jonathan's comments? Thanks!

Created attachment 173272 [details] [review]
Bug 37148 - Move hold to pull column filters to top

This bug moves the column filters on the holds to pull page (pendingreserves.pl)
to the table header, similar to how it is being done on shelves.pl. This
should make it much clearer to users that filtering is possible, and also
make the filters float on the page, so they are always visible.

To test:
a)  apply half a dozen holds, with varying item types, shelving locations,
    holding branches, titles, and collections
b)  go to pendingreserves.pl
c)  notice that the filters are in the table's footer, and that you have
    to scroll down to see them
d)  APPLY PATCH
e)  notice that on pendingreserves.pl, the filters present in tfoot are
    now in thead
f)  notice that the filters are unchanged, aside from location on page
g)  notice that the filters now stick to the top of the page, like the
    headings did
h)  notice that the headings still change the sort order when clicked
i)  notice that the filter inputs all still work as expected
j)  SIGN OFF! :-)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

(In reply to Jonathan Druart from comment #19)
> +    [% IF print %]
> +        <script>
> +            $( document ).ready(function() {
> +                window.print();
> +                window.onafterprint = function () {+                   
> window.close();
> +                }
> +                setTimeout('window.close()', 1000); // Hack for Chrome < 63
> +            });
> +        </script>
> +    [% END #/print %]
> 
> Why do we have this?

I don't think we do anymore, actually. I think it is obsolete, so I have removed it. Thanks :-)

Jake.