Summary: | opacsmallimage doesn't work in ccsr theme | ||
---|---|---|---|
Product: | Koha | Reporter: | Nicole C. Engard <nengard> |
Component: | OPAC | Assignee: | Martin Renvoize (ashimema) <martin.renvoize> |
Status: | CLOSED FIXED | QA Contact: | Jonathan Druart <jonathan.druart> |
Severity: | major | ||
Priority: | P5 - low | CC: | chris, edward.m.veal, jonathan.druart, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Attachments: |
Bug 8883 - Fix for opacsmallimage in ccsr theme.
[SIGNED-OFF] Bug 8883: 'opacsmallimage' doesn't work in ccsr theme Bug 8883: 'opacsmallimage' doesn't work in ccsr theme |
Description
Nicole C. Engard
2012-10-05 18:53:21 UTC
This might help in finding all the preferences that are supposed to work on the themes: http://manual.koha-community.org/3.10/en/impopac.html#editableopac Nicole Changing assignee to Frédérick since he originally designed the theme and can probably fix this. Created attachment 14334 [details] [review] Bug 8883 - Fix for opacsmallimage in ccsr theme. Does what is says on the tin.. However, it's not a perfect patch. I tried to follow the coding guidelines to the letter (found http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1:_Template_Toolkit_markup_inside_HTML) but found it difficult to implement the patch without breaking Rule 1 in some way. If anyone has a better way of working around this, then let me know? Created attachment 14340 [details] [review] [SIGNED-OFF] Bug 8883: 'opacsmallimage' doesn't work in ccsr theme This patch makes it possible to customize the CCSR theme using the opacsmallimage system preference. It also enables the LibraryName preference for opacsmallimage alt text. To test, set the opacsmallimage preference and switch to the CCSR theme. Your custom image should appear. Set the LibraryName preference and confirm that the alt attribute of the custom logo changes accordingly. Signed-off-by: Owen Leonard <oleonard@myacpl.org> This patch works correctly, although it lacked a description and test plan, which I have added. I would have used IF instead of UNLESS because to me that seems more readable, but it's not a deal-breaker. (In reply to comment #4) > However, it's not a perfect patch. I tried to follow the coding guidelines > to the letter (found > http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML1: > _Template_Toolkit_markup_inside_HTML) but found it difficult to implement > the patch without breaking Rule 1 in some way. Your patch doesn't break Rule 1. Rule 1 refers specifically to extended constructions like IF...ELSE inside a tag, not to single variable declarations. Great.. thanks for the clarifications Owen! Also, happy to switch around the UNLESS/IF's.. I couldn't see any clear motive for doing it either way or indeed a current standard. Actually, I agree and prefer an IF to an UNLESS, but I threw it together using code initially derived from the prog templates.. which used an UNLESS so it stuck.. ;) QA comment: This patch allows to use the opacsmallimage pref from the ccsr theme. Works as expected. Marked as Passed QA. Created attachment 14599 [details] [review] Bug 8883: 'opacsmallimage' doesn't work in ccsr theme This patch makes it possible to customize the CCSR theme using the opacsmallimage system preference. It also enables the LibraryName preference for opacsmallimage alt text. To test, set the opacsmallimage preference and switch to the CCSR theme. Your custom image should appear. Set the LibraryName preference and confirm that the alt attribute of the custom logo changes accordingly. Signed-off-by: Owen Leonard <oleonard@myacpl.org> This patch works correctly, although it lacked a description and test plan, which I have added. I would have used IF instead of UNLESS because to me that seems more readable, but it's not a deal-breaker. Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> This patch has been pushed to master. Pushed to 3.10.x will be in 3.10.2 |