Bug 6616 - Move common table pager markup to include
Summary: Move common table pager markup to include
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: PATCH-Sent (DO NOT USE) enhancement (vote)
Assignee: Owen Leonard
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks: 6689
  Show dependency treegraph
 
Reported: 2011-07-20 16:50 UTC by Owen Leonard
Modified: 2013-12-05 20:01 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed fix (18.82 KB, patch)
2011-07-20 18:30 UTC, Owen Leonard
Details | Diff | Splinter Review
Revised fix (18.72 KB, patch)
2011-07-20 18:46 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Fix for Bug 6616 - Move common table pager markup to include (18.85 KB, patch)
2011-07-27 20:33 UTC, Katrin Fischer
Details | Diff | Splinter Review
Revised fix (18.00 KB, patch)
2011-10-06 13:22 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF]Fix for Bug 6616 - Move common table pager markup to include (18.08 KB, patch)
2011-10-06 13:37 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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