Bug 9130

Summary: Remove old javascript from Parcel
Product: Koha Reporter: David Cook <dcook>
Component: AcquisitionsAssignee: David Cook <dcook>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, julian.maurice, kyle.m.hall, kyle
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8028
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 9130 - Remove old javascript from Parcel
[SIGNED-OFF] Bug 9130 - Remove old javascript from Parcel
Bug 9130 follow up: add statesave cookie and "all" option for parcel drop-down menu
[SIGNED-OFF] Bug 9130 follow up: add statesave cookie and "all" option for parcel drop-down menu
Bug 9130 - Remove old javascript from Parcel
Bug 9130 follow up: add statesave cookie and "all" option for parcel drop-down menu
Bug 9130 - Remove old javascript from Parcel
Bug 9130 follow up: add statesave cookie and "all" option for parcel drop-down menu

Description David Cook 2012-11-21 21:05:57 UTC
When receiving orders, the old javascript would show the following above the tables for pending orders and received orders:

"Only the first 5 items are displayed. Click here to show all 'X' items."

With the implementation of the Jquery Datatables plugin, this text and the 4 JS functions associated with it are now pointless (since users should be using the datatable toolbar) and problematic.

The problem is that the Jquery toolbar says "Showing 1 to 10 of X", while in fact it is actually only showing 5 items, because that other JS takes place after the Jquery. 

While not a crippling bug, it's certainly irritating and misleading.
Comment 1 David Cook 2012-11-21 21:20:22 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2012-11-22 16:32:04 UTC Comment hidden (obsolete)
Comment 3 Julian Maurice 2012-11-22 16:34:28 UTC
(In reply to comment #2)
> While not a crippling bug, it's certainly irritating and misleading.

Yes it is! Thank you for the patch ;)
Comment 4 Jonathan Druart 2012-11-23 11:17:25 UTC
I add Kyle to the CC list of this bug.

Do you agree with the current behaviour ?
Comment 5 Kyle M Hall 2012-12-03 17:16:38 UTC
(In reply to comment #4)
> I add Kyle to the CC list of this bug.
> 
> Do you agree with the current behaviour ?

I would tend to agree with you here. I do have one question. Does the table have a pulldown to choose the number of results displayed? If so, can we make that 'sticky' via a cookie instead. That way, the functionality isn't totally lost. I don't have a Koha installation set up for me to look at right now, or I could probably answer the question myself.
Comment 6 David Cook 2012-12-04 01:00:25 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2012-12-04 13:03:06 UTC
Fantastic! Good work!

Kyle

(In reply to comment #6)
> Created attachment 13866 [details] [review]
> Bug 9130 follow up: add statesave cookie and "all" option for parcel
> drop-down menu
> 
> This patch sets the bStateSave parameter for the "Pending orders"  and
> "Already received" Datatables in Acquisitions, which uses a cookie to save
> the "pagination information, display length, filtering and sorting".
> 
> It also sets the iCookieDuration parameter to 1000 days (the default value
> is 2 hours otherwise).
> 
> It also adds an "all" option to the dro-down menu using the aLengthMenu
> parameter.
> 
> --
> 
> Kyle, this should preserve that "sticky" functionality of which you spoke :)
Comment 8 Kyle M Hall 2012-12-04 13:03:52 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2012-12-04 13:56:34 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2012-12-04 13:57:55 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2012-12-04 13:59:00 UTC
Thanks David for your followup.

1 little remark: when you add a value (60*60*24*1000), could you add directly the result (86400000)? Because like this the calculation is not performed each time.
It is JS code so it is not crucial and not blocker.

Marked as Passed QA.
Comment 12 Jonathan Druart 2012-12-04 14:00:13 UTC
Created attachment 13875 [details] [review]
Bug 9130 - Remove old javascript from Parcel

When receiving orders, the old javascript would show the following above
the tables for pending orders and received orders:

"Only the first 5 items are displayed. Click here to show all 'X'
items."

With the implementation of the Jquery Datatables plugin, this text and
the 4 JS functions associated with it are now pointless (since users
should be using the datatable toolbar) and problematic.

The problem is that the Jquery toolbar says "Showing 1 to 10 of X",
while in fact it is actually only showing 5 items, because that other JS
takes place after the Jquery.

While not a crippling bug, it's certainly irritating and misleading.

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 13 Jonathan Druart 2012-12-04 14:00:19 UTC
Created attachment 13876 [details] [review]
Bug 9130 follow up: add statesave cookie and "all" option for parcel drop-down menu

This patch sets the bStateSave parameter for the "Pending orders"  and "Already received" Datatables in Acquisitions, which uses a cookie to save the "pagination information, display length, filtering and sorting".

It also sets the iCookieDuration parameter to 1000 days (the default value is 2 hours otherwise).

It also adds an "all" option to the dro-down menu using the aLengthMenu parameter.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 14 Jared Camins-Esakov 2012-12-05 13:54:05 UTC
This patch has been pushed to master.
Comment 15 David Cook 2012-12-05 19:05:59 UTC
(In reply to comment #11)
> Thanks David for your followup.
> 
> 1 little remark: when you add a value (60*60*24*1000), could you add
> directly the result (86400000)? Because like this the calculation is not
> performed each time.
> It is JS code so it is not crucial and not blocker.
> 
> Marked as Passed QA.

Good point, Jonathan. In retrospect, I could've just added the result. When looking through other examples (like in jquery.cookie.min.js), I noticed them writing out the whole calculation, so I just copied that at the time. I don't know if it's worth a follow-up, but certainly something to keep in mind for future Datatable mods.