Bug 8883 - opacsmallimage doesn't work in ccsr theme
Summary: opacsmallimage doesn't work in ccsr theme
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Martin Renvoize
QA Contact: Jonathan Druart
Depends on:
Reported: 2012-10-05 18:53 UTC by Nicole C. Engard
Modified: 2014-05-26 21:04 UTC (History)
4 users (show)

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

Bug 8883 - Fix for opacsmallimage in ccsr theme. (1.30 KB, patch)
2012-12-31 13:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8883: 'opacsmallimage' doesn't work in ccsr theme (1.98 KB, patch)
2012-12-31 15:15 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 8883: 'opacsmallimage' doesn't work in ccsr theme (1.99 KB, patch)
2013-01-15 13:15 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2012-10-05 18:53:21 UTC
In the CCSR theme if you have the opacsmallimage preference set it doesn't do anything. Many libraries use this preference for their own logos and all themes should use it.

Comment 1 Nicole C. Engard 2012-10-05 18:54:59 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 

Comment 2 Nicole C. Engard 2012-12-19 20:06:13 UTC
Changing assignee to Frédérick since he originally designed the theme and can probably fix this.
Comment 3 Nicole C. Engard 2012-12-26 13:40:15 UTC
Assignee changed per comment on bug 8884.
Comment 4 Martin Renvoize 2012-12-31 13:43:02 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2012-12-31 15:15:38 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2012-12-31 15:17:48 UTC
(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.
Comment 7 Martin Renvoize 2012-12-31 15:44:04 UTC
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.. ;)
Comment 8 Jonathan Druart 2013-01-15 13:14:52 UTC
QA comment:

This patch allows to use the opacsmallimage pref from the ccsr theme.

Works as expected.

Marked as Passed QA.
Comment 9 Jonathan Druart 2013-01-15 13:15:05 UTC
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

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>
Comment 10 Jared Camins-Esakov 2013-01-16 01:04:31 UTC
This patch has been pushed to master.
Comment 11 Chris Cormack 2013-01-16 05:51:08 UTC
Pushed to 3.10.x will be in 3.10.2