Description
Chris Cormack
2010-05-21 00:57:37 UTC
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 ? |