---- Reported by gmcharlt@gmail.com 2008-12-31 08:18:29 ---- When styling a table to use different shading for alternating rows, templates should test the H::T::Pro loop context variable __odd__ instead of checking the value of a toggle variable set by a script. For example: <!-- TMPL_UNLESS NAME="__odd__" --><tr class="highlight"><!-- TMPL_ELSE --><tr><!-- /TMPL_UNLESS --> instead of <!-- TMPL_IF NAME="toggle" --><tr class="highlight"><!-- TMPL_ELSE --><tr><!-- /TMPL_IF --> To make this cleanup for a given script, both the script and the template should be modified. As of 2008-12-31, affected scripts are: acqui/spent.pl acqui/booksellers.pl acqui/parcel.pl admin/authorised_values.pl admin/aqbudget.pl admin/branches.pl admin/marc_subfields_structure.pl admin/authtypes.pl admin/auth_subfields_structure.pl admin/currency.pl admin/koha2marclinks.pl admin/letter.pl admin/itemtypes.pl admin/cities.pl admin/aqbookfund.pl admin/auth_tag_structure.pl admin/stopwords.pl admin/smart-rules.pl admin/marctagstructure.pl admin/z3950servers.pl admin/roadtype.pl admin/categorie.pl admin/biblio_framework.pl C4/SIP/ILS/Patron.pm cataloguing/addbooks.pl cataloguing/z3950_search.pl labels/pcard-member-search.pl members/boraccount.pl members/moremember.pl members/member.pl opac/opac-user.pl reports/reservereport.pl serials/distributedto.pl serials/acqui-search-result.pl tools/overduerules.pl ---- Additional Comments From joe.atzberger@liblime.com 2009-02-05 11:42:47 ---- Note, one could also implement this with jquery: $("tr:even").css("background-color", "#bbbbff"); In fact, that's the textbook example: http://docs.jquery.com/Selectors/even ---- Additional Comments From oleonard@myacpl.org 2009-02-05 12:28:16 ---- It would be nice to have a solution that was a one-off, but I'd much rather modify the template and avoid the processing burden like we see with the jquery table-sorter. ---- Additional Comments From mjr@ttllp.co.uk 2009-06-12 17:22:17 ---- patches appearing on list from Garry Collum <gcollum@gmail.com> - is this bug his now? Anyone co-ordinating the effort? ---- Additional Comments From joe.atzberger@liblime.com 2009-06-12 17:51:13 ---- I think Gary can have the bug if he wants it. ---- Additional Comments From gcollum@gmail.com 2009-07-04 23:00:40 ---- Multiple patches submitted. Toggle variables still appear in these files. C4/Search.pm C4/Serials.pm C4/Acquisition.pm C4/SIP/ILS/Patron.pm koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tmpl koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tmpl koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-home.tmpl koha-tmpl/intranet-tmpl/prog/en/modules/z3950/searchresult.tmpl members/boraccount.pl opac/opac-user.pl reports/reservereport.pl serials/acqui-search-result.pl serials/serials-home.pl ---- Additional Comments From gcollum@gmail.com 2009-07-23 03:50:09 ---- Created an attachment Patch for serials-home.tmpl ---- Additional Comments From gcollum@gmail.com 2009-07-23 03:55:33 ---- Created an attachment Patch for serials acqui-search-result The listed files in serials are done. subscription-add.tmpl and serials-home.pl have no toggle variables, just the word toggle in remarks. ---- Additional Comments From gcollum@gmail.com 2009-10-13 00:23:51 ---- Created an attachment Patch for dictionary.tmpl ---- Additional Comments From colin.campbell@ptfs-europe.com 2010-03-26 16:53:37 ---- Created an attachment Patch for boraccount.pl --- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:57 UTC --- This bug was previously known as _bug_ 2889 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2889 Imported an attachment (id=911) Imported an attachment (id=912) Imported an attachment (id=913) Imported an attachment (id=914) Actual time not defined. Setting to 0.0 CC member joe.atzberger@liblime.com does not have an account here CC member mjr@ttllp.co.uk does not have an account here The original submitter of attachment 911 [details] [review] is unknown. Reassigning to the person who moved it here: chris@bigballofwax.co.nz. The original submitter of attachment 912 [details] [review] is unknown. Reassigning to the person who moved it here: chris@bigballofwax.co.nz. The original submitter of attachment 913 [details] [review] is unknown. Reassigning to the person who moved it here: chris@bigballofwax.co.nz. The original submitter of attachment 914 [details] [review] is unknown. Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
Created attachment 2157 [details] [review] Patch for serials-collection.tmpl
Pushed serials-collection.tmpl patch.
Created attachment 2306 [details] [review] Proposed patch for report tmpl's.
Created attachment 2307 [details] [review] Proposed patch for guided reports.
Instead of marking this invalid shouldn't we re-word it to apply to Template::Toolkit?
(In reply to comment #5) I suppose so. We could do something like this: http://www.robertprice.co.uk/robblog/archive/2004/9/Alternating_Table_Rows_With_Template_Toolkit_And_CSS.shtml Or even better, something like Tablecloth ( http://cssglobe.com/lab/tablecloth/ ), so we could just add a css attribute to any table that needs striping and have it handled automatically. > Instead of marking this invalid shouldn't we re-word it to apply to > Template::Toolkit?
Hi Owen, should we perhaps move this to a separate bug or include a best practice for 'table striping' on a page in the wiki? I am not sure how this is done right now.
Owen, could you take a look and check if this bug is still valid?
This bug is still valid. I see many instances with something like this: [% IF ( user.toggle ) %] <tr> [% ELSE %] <tr class="highlight"> [% END %] These can be converted to use [% IF ( loop.odd ) %] (which is already used in many places in the templates) or eliminated because the table is being manipulated by the DataTables plugin.
Created attachment 30071 [details] [review] Patch updates template acqui/aqbasketsearch.tt
Created attachment 30790 [details] [review] Bug 2889 - templates should use [% IF ( loop.odd ) %] - Administration Some scripts pass a template variable to facilitate an alternate table row class for styling. Other use the 'IF (loop.odd)' construction. I think we're at the point where the CSS3 :nth-child() selector is widely-supported enough that we can do without template-based solutions: http://caniuse.com/#feat=css-sel3 This patch adds such a selector to the staff client CSS and removes the corresponding template markup from Administration pages. The last in this series of patches will remove the redundant CSS. Also in this patch: a few minor markup corrections. To test, apply the patch and clear your browser cache if necessary. View the following pages and confirm that alternate table row highlighting works as before: - Administration -> Budgets - Acquisitions -> Vendor -> Contracts - Administration -> Budgets -> Budget -> Planning - Administration -> Authority types - Administration -> Authority types -> MARC structure - Administration -> Authority types -> MARC structure -> subfields - Administration -> MARC bibliographic framework - Administration -> MARC bibliographic framework -> MARC structure - Administration -> MARC bibliographic framework -> MARC structure -> Subfields - Administration -> Libraries and groups - Administration -> Cities and towns - Administration -> Classification sources - Administration -> Circulation and fines rules - Administration -> Currencies and exchange rates - Administration -> Item types - Administration -> Koha to MARC mapping - Administration -> System preferences -> Local use - Administration -> Z39.50 client targets
Applying: BUG: 2889 template aqbasketusersearch add loop.odd Applying: Bug 2889 - templates should use [% IF ( loop.odd ) %] - Administration Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/z3950servers.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/currency.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/currency.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/branch_transfer_limits.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/auth_subfields_structure.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css Failed to merge in the changes. Patch failed at 0001 Bug 2889 - templates should use [% IF ( loop.odd ) %] - Administration When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-2889---templates-should-use--IF--loopodd-----A-zUC7yD.patch nick@debian:~/koha$
Created attachment 31834 [details] [review] Bug 2889 - templates should use [% IF ( loop.odd ) %] - Administration Some scripts pass a template variable to facilitate an alternate table row class for styling. Other use the 'IF (loop.odd)' construction. I think we're at the point where the CSS3 :nth-child() selector is widely-supported enough that we can do without template-based solutions: http://caniuse.com/#feat=css-sel3 This patch adds such a selector to the staff client CSS and removes the corresponding template markup from Administration pages. The last in this series of patches will remove the redundant CSS. Also in this patch: a few minor markup corrections. To test, apply the patch and clear your browser cache if necessary. View the following pages and confirm that alternate table row highlighting works as before: - Administration -> Budgets - Acquisitions -> Vendor -> Contracts - Administration -> Budgets -> Budget -> Planning - Administration -> Authority types - Administration -> Authority types -> MARC structure - Administration -> Authority types -> MARC structure -> subfields - Administration -> MARC bibliographic framework - Administration -> MARC bibliographic framework -> MARC structure - Administration -> MARC bibliographic framework -> MARC structure -> Subfields - Administration -> Libraries and groups - Administration -> Cities and towns - Administration -> Classification sources - Administration -> Circulation and fines rules - Administration -> Currencies and exchange rates - Administration -> Item types - Administration -> Koha to MARC mapping - Administration -> System preferences -> Local use - Administration -> Z39.50 client targets
Hi Owen, These are the screens where the alternate highlighting did not work for me: Authority types Authority types -> MARC structure -> subfields Administration -> MARC bibliographic framework Administration -> MARC bibliographic framework -> MARC structure -> Subfields Administration -> Classification sources Administration -> Circulation and fines rules (Doesn't work in circ table but does in all below) Administration -> Koha to MARC mapping I double checked on master and they were working before the patch.
(In reply to Nick Clemens from comment #14) > These are the screens where the alternate highlighting did not work for me Did you clear your browser cache? The patch updates the staff client CSS.
Patch tested with a sandbox, by Nick Clemens <nick@quecheelibrary.org>
Created attachment 31858 [details] [review] BUG: 2889 template aqbasketusersearch add loop.odd To Test 1/ In Acquisitions > Create a test Budget 2/ In the Budget > Create at least three Test Funds 3/ Create at least three Users who use the fund 4/ Select Search for Basket User 5/ In the Table results list see the list of users and see the alternate table highlighting 6/ Apply the patch 7/ Refresh the Search for Basket User results window 8/ In the Table results list see the list of users and see the alternate table highlighting in place 9/ Bug fixed Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 31859 [details] [review] Bug 2889 - templates should use [% IF ( loop.odd ) %] - Administration Some scripts pass a template variable to facilitate an alternate table row class for styling. Other use the 'IF (loop.odd)' construction. I think we're at the point where the CSS3 :nth-child() selector is widely-supported enough that we can do without template-based solutions: http://caniuse.com/#feat=css-sel3 This patch adds such a selector to the staff client CSS and removes the corresponding template markup from Administration pages. The last in this series of patches will remove the redundant CSS. Also in this patch: a few minor markup corrections. To test, apply the patch and clear your browser cache if necessary. View the following pages and confirm that alternate table row highlighting works as before: - Administration -> Budgets - Acquisitions -> Vendor -> Contracts - Administration -> Budgets -> Budget -> Planning - Administration -> Authority types - Administration -> Authority types -> MARC structure - Administration -> Authority types -> MARC structure -> subfields - Administration -> MARC bibliographic framework - Administration -> MARC bibliographic framework -> MARC structure - Administration -> MARC bibliographic framework -> MARC structure -> Subfields - Administration -> Libraries and groups - Administration -> Cities and towns - Administration -> Classification sources - Administration -> Circulation and fines rules - Administration -> Currencies and exchange rates - Administration -> Item types - Administration -> Koha to MARC mapping - Administration -> System preferences -> Local use - Administration -> Z39.50 client targets Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 31866 [details] [review] Bug 2889: template aqbasketusersearch add loop.odd To Test 1/ In Acquisitions > Create a test Budget 2/ In the Budget > Create at least three Test Funds 3/ Create at least three Users who use the fund 4/ Select Search for Basket User 5/ In the Table results list see the list of users and see the alternate table highlighting 6/ Apply the patch 7/ Refresh the Search for Basket User results window 8/ In the Table results list see the list of users and see the alternate table highlighting in place 9/ Bug fixed Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 31867 [details] [review] Bug 2889 - templates should use [% IF ( loop.odd ) %] - Administration Some scripts pass a template variable to facilitate an alternate table row class for styling. Other use the 'IF (loop.odd)' construction. I think we're at the point where the CSS3 :nth-child() selector is widely-supported enough that we can do without template-based solutions: http://caniuse.com/#feat=css-sel3 This patch adds such a selector to the staff client CSS and removes the corresponding template markup from Administration pages. The last in this series of patches will remove the redundant CSS. Also in this patch: a few minor markup corrections. To test, apply the patch and clear your browser cache if necessary. View the following pages and confirm that alternate table row highlighting works as before: - Administration -> Budgets - Acquisitions -> Vendor -> Contracts - Administration -> Budgets -> Budget -> Planning - Administration -> Authority types - Administration -> Authority types -> MARC structure - Administration -> Authority types -> MARC structure -> subfields - Administration -> MARC bibliographic framework - Administration -> MARC bibliographic framework -> MARC structure - Administration -> MARC bibliographic framework -> MARC structure -> Subfields - Administration -> Libraries and groups - Administration -> Cities and towns - Administration -> Classification sources - Administration -> Circulation and fines rules - Administration -> Currencies and exchange rates - Administration -> Item types - Administration -> Koha to MARC mapping - Administration -> System preferences -> Local use - Administration -> Z39.50 client targets Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
IMO last commit message should be "templates should *not* use".
Arg, I didn't update the status deliberately.
Patches pushed to master. Thanks Karl and Owen!
We should also remove [% IF ( loop.even ) %] right ?