Bug 6616

Summary: Move common table pager markup to include
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: PATCH-Sent (DO NOT USE) CC: chris, katrin.fischer, koha.sekjal, paul.poulain
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 6689    
Attachments: Proposed fix
Revised fix
[SIGNED-OFF] Fix for Bug 6616 - Move common table pager markup to include
Revised fix
[SIGNED-OFF]Fix for Bug 6616 - Move common table pager markup to include

Description Owen Leonard 2011-07-20 16:50:10 UTC
The table pager markup used in conjunction with the tablesorter javascript is virtually the same in each template which uses it. I propose a new include file to contain it. We can use T:T's ability to define variables in an [% INCLUDE %] directive to pass the default page size in each case:

[% INCLUDE 'table-pager.inc' perpage='20' %]
Comment 1 Owen Leonard 2011-07-20 18:30:44 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2011-07-20 18:46:18 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2011-07-27 20:33:20 UTC Comment hidden (obsolete)
Comment 4 Katrin Fischer 2011-07-27 20:36:20 UTC
Signed off, works as expected.

A note: 
The plugin shows current page/pages total in an input box. You can edit this box and put something else in there. For example, change 1/12 into 3/12. 
This will not work.

 Perhaps the styling could be changed so the box shows not as a normal input field that you can edit.
Comment 5 Paul Poulain 2011-10-06 12:44:57 UTC
QA comment

Seems there is a problem with the patch:

14:43 ~/koha.dev/koha-community (BZ6616 $%)$ git bz apply 6616
Bug 6616 - Move common table pager markup to include

[SIGNED-OFF] Fix for Bug 6616 - Move common table pager markup to include
Apply? [yn] y

fatal: cannot convert from UTF-8utf-8 to UTF-8


Looking at it, there's something strange:
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"


the both text/plain strings are probably the cause of the problem.Marking failed QA

Otherwise, i had been happy to mark passed QA, it's a html-only patch
Comment 6 Owen Leonard 2011-10-06 13:22:24 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2011-10-06 13:27:51 UTC
This is probably caused by something in my installation - it only occurs sometimes in not in general. Would appreciate any hints how to fix that.
Can we say the sign-off still stands if the only difference between original patch and signed-off is that?
Comment 8 Paul Poulain 2011-10-06 13:37:30 UTC
Created attachment 5738 [details] [review]
[SIGNED-OFF]Fix for Bug 6616 - Move common table pager markup to include

Created new include for the tablesorter pager markup:

[% INCLUDE 'table-pager.inc' perpage='20' %]

'perpage' variable defines the default items per page for
the paged table.

Unique changes in branch_transfer_limits.tt:

- Moved tablesorter configuration out of a loop and changed
  it to work by class instead of id (less repeated markup).
- Corrected an errant TMPL tag.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Comment 9 Paul Poulain 2011-10-06 13:38:22 UTC
QA comment:

patch now applies, template only change, nothing to say from a QA pov
Comment 10 Ian Walls 2011-10-06 19:13:15 UTC
Template only change, increases consistency and reusability.  Issue with the
field being editable, but unusable, can be moved to a separate bug report
(solved either by making it work, or making it read only).

Marking as Passed QA
Comment 11 Owen Leonard 2011-10-06 19:34:44 UTC
The patch for Bug 6689 will include a cosmetic fix to address the editable field issue.
Comment 12 Chris Cormack 2011-10-09 07:08:58 UTC
Pushed, please test