Bug 8028 - Make table collapsing on parcel.pl sticky
Summary: Make table collapsing on parcel.pl sticky
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-30 15:34 UTC by Kyle M Hall
Modified: 2013-05-23 06:24 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8028 - Make table collapsing on parcel.pl sticky (3.36 KB, patch)
2012-04-30 15:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8028 - Make table collapsing on parcel.pl sticky (3.64 KB, patch)
2012-07-12 14:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8028 - Make table collapsing on parcel.pl sticky - Hide link for 5 or fewer items. (1.24 KB, patch)
2012-07-16 17:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8028 - Make table collapsing on parcel.pl sticky - Hide link for 5 or fewer items. (1.31 KB, patch)
2012-07-18 08:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8028 - Followup (1.16 KB, patch)
2012-09-07 16:17 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2012-04-30 15:34:15 UTC
The tables on parcel.pl default to show only the first five items in the table, and librarians must click the a link to display all the items. Every time the page is reloaded, the table gets re-collapsed, and the librarian must again click the link. Parcel.pl should remember which way the table is set and keep it that way until it is changed again.
Comment 1 Kyle M Hall 2012-04-30 15:36:23 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-04-30 15:41:36 UTC
Hi Kyle, wouldn't it work to pass around the setting instead of using cookies?
Comment 3 Kyle M Hall 2012-05-02 13:56:26 UTC
(In reply to comment #2)
> Hi Kyle, wouldn't it work to pass around the setting instead of using
> cookies?

That would work for refreshing the page, but not for moving from say parcel.pl to the item details page and back. It could be done, but would require far more code and I don't see any advantage to that.

The alternative would be a system preference, but then the librarians would be in the opposite situation, where if they wanted it collapsed, they'd have to click the link every time.
Comment 4 Jonathan Druart 2012-07-12 14:38:19 UTC
Created attachment 10797 [details] [review]
Bug 8028 - Make table collapsing on parcel.pl sticky

The tables on parcel.pl default to show only the first
five items in the table, and librarians must click the
a link to display all the items. Every time the page is
reloaded, the table gets re-collapsed, and the librarian
must again click the link. Parcel.pl should remember
which way the table is set and keep it that way until
it is changed again.

This is accomplished by adding two cookies for this page,
one for each table. This cookie stores the state of the
table ( collapsed, or uncollapsed ) and sets each table
to the correct state on each page load.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Works as expected.

But a "funny" behaviour, if I have only 1 item (or < 6):
  All 1 items are displayed. Click here to show only the first 5 items.
and
  Only the first 5 items are displayed. Click here to show all 1 items.
Comment 5 Kyle M Hall 2012-07-13 13:54:10 UTC
If anyone has a suggestion for a good way to word this, I will write a patch to update the language.
Comment 6 Jonathan Druart 2012-07-16 10:02:12 UTC
Kyle, Thanks for signed-off !

> If anyone has a suggestion for a good way to word this, I will write a patch to update the language.

I think we can just check if the number of items are > 5 to display links.
Comment 7 Kyle M Hall 2012-07-16 17:11:42 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2012-07-18 08:57:01 UTC
Created attachment 10926 [details] [review]
Bug 8028 - Make table collapsing on parcel.pl sticky - Hide link for 5 or fewer items.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 9 Paul Poulain 2012-08-02 13:55:58 UTC
QA comment: I'm unable to understand & reproduce the problem and the solution. Could you give me more explanations ? A screenshot ?

2 comments (that will maybe be removed when I understand & reproduce):
 * is using a cookie the best way to do that ?
 * isn't DataTable jquery plugin what we should use ?
Comment 10 Kyle M Hall 2012-08-03 11:19:48 UTC
(In reply to comment #9)
> QA comment: I'm unable to understand & reproduce the problem and the
> solution. Could you give me more explanations ? A screenshot ?

Currently, if there are more than 5 items in the table on parcel.pl, it only shows you the first 5. You must click a link to show more than 5. This patch makes the page 'remember' if you would prefer to see just the first 5, or more than 5.

> 2 comments (that will maybe be removed when I understand & reproduce):
>  * is using a cookie the best way to do that ?

I think it is the best way to accomplish this. The idea is to give the librarians the choice. A system preference would force all the librarians to have the same behavior ( 5 or more than 5 ). Additionally, parameter passing won't work without adding the parameter to *every* page in Koha, which sounds like a far worse solution.

>  * isn't DataTable jquery plugin what we should use ?

Nobody as implemented DataTables here, and it is outside the scope of this bug.
Comment 11 Ian Walls 2012-08-17 15:20:13 UTC
Adds a couple small cookies for stickiness, and some logic to handle them.  Followup patch fixes conditional nesting.  Marking Passed QA.
Comment 12 Paul Poulain 2012-08-31 21:31:52 UTC
mmm... I made a basket with 6 lines, and had another one with 2 lines pending. When I reach the parcel.pl page, I see the 8 pending lines directly, and no link to collapse/expand.
Am I missing something ?

(I recieved 4 of them, and saw no change)
Comment 13 Paul Poulain 2012-09-07 13:31:46 UTC
Kyle, ping on my comment 12. I need to test that before pushing !
Comment 14 Kyle M Hall 2012-09-07 16:17:44 UTC
Created attachment 12059 [details] [review]
Bug 8028 - Followup
Comment 15 Kyle M Hall 2012-09-07 16:18:27 UTC
(In reply to comment #12)
> mmm... I made a basket with 6 lines, and had another one with 2 lines
> pending. When I reach the parcel.pl page, I see the 8 pending lines
> directly, and no link to collapse/expand.
> Am I missing something ?
> 
> (I recieved 4 of them, and saw no change)

Good catch. I've attach a small followup to fix the issue.
Comment 16 Paul Poulain 2012-09-12 15:35:18 UTC
This time it's OK, patch pushed to master !