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' %]
Created attachment 4686 [details] [review] Proposed fix 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.
Created attachment 4687 [details] [review] Revised fix Revised fix moves div with "pager" class to the include to allow consistent styling.
Created attachment 4757 [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> Checked: - Home › Administration › Authority MARC Framework: Default Framework - Home › Administration › Authorized values - Home › Administration › Set Library Checkin and Transfer Policy - Home › Administration › Libraries and Groups - Home › Administration › Patron Categories - Home › Administration › Cities - Home › Administration › Item Types Administration - Home › Administration › MARC Frameworks › Default MARC framework - Home › Reports › Guided Reports Wizard › Saved Reports - Home › Patrons › Circulation History for Koha Admin No problems found. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
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.
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
Created attachment 5737 [details] [review] Revised fix Looks like a problem in patch formatting. I've rebased the patch against current master and reformatted it.
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?
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>
QA comment: patch now applies, template only change, nothing to say from a QA pov
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
The patch for Bug 6689 will include a cosmetic fix to address the editable field issue.
Pushed, please test