Libraries with multiple branches should be able to change look&feel of OPAC using branch specific CSS (e.g. in in OPACUserCSS) This can be done using a branch specific class in the body tag (class="branchcode") That would be something like <body id="opac-main" class="CPL"> Using this class, we could style branch specific pages e.g. in OPACUserCSS: /*Background for all branches*/ body { background-color: red; } /*Yellow background and pink border for branch with code CPL*/ body.CPL { background-color: yellow; border: 10px solid pink; } /*Green background and red border for branch with code CPL*/ body.FFL { background-color: green; border: 10px solid red; } I did some tests as a proof of concept. Patch follows.
Created attachment 27784 [details] [review] [Proof of concept] Bug 12162 - Add class="branchcode" to body tag to make OPAC CSS-styleable per branch This patch implements branch specific CSS for Opac Main page and OPAC Search. If the proof of concept works out, other Opac pages can be adapted. To test: Apply patch Edit system preference OPACUserCSS and insert something like /*Background for all branches*/ body { background-color: red; } /*Yellow background and pink border for branch with code CPL*/ body.CPL { background-color: yellow; border: 10px solid pink; } /*Green background and red border for branch with code FFL*/ body.FFL { background-color: green; border: 10px solid red; } Go to Opac. Switch between branches CPL and FFL, see how background color changes...
I like this idea. It also would allow you to use jquery per branch, which I like.
Good idea, but perhaps it should be something like class="branch-FOO", just in case the branch short name FOO would match another css class.
(In reply to paxed from comment #3) > Good idea, but perhaps it should be something like class="branch-FOO", just > in case the branch short name FOO would match another css class. Good point!
Created attachment 27811 [details] [review] Bug 12162 - Add class="branchcode" to body tag to make OPAC CSS-styleable per branch This patch implements branch specific CSS for Opac Main page and OPAC Search. If the proof of concept works out, other Opac pages can be adapted. To test: Apply patch Edit system preference OPACUserCSS and insert something like /*Background for all branches*/ body { background-color: red; } /*Yellow background and pink border for branch with code CPL*/ body.branch-CPL { background-color: yellow; border: 10px solid pink; } /*Green background and red border for branch with code FFL*/ body.branch-FFL { background-color: green; border: 10px solid red; } Go to Opac. Switch between branches CPL and FFL, see how background color changes... Changes to previous patch: New version using branch-FOO as class name. Prog theme added for this proof of concept.
Should this be "needs signoff"?
(In reply to paxed from comment #6) > Should this be "needs signoff"? No, not yet. It's just a proof of concept (woudl appreciate tests, though). Many files are involved. So I think it would be good get it done in a short timeframe to prevent to much cases of "does not apply". And it should be done for bootstrap theme only, as prog theme is about to be depractated. Comments are welcome!
I really like the idea of this. Additionally to the branchcode it would be nice to have the currently selected language. That would allow to trigger the visibility of text with CSS, while you have to use jQuery to do that right now. Looking at the code I wonder if we could put the logic into Auth.pm instead, so that the logic doesn't need to be repeated in all the .pl files. We already do that for some other variables and system preferences.
Perhaps the body-tag should be generated in an include file, so it could be INCLUDEd or PROCESSed - like doc-head-open.inc and doc-head-close.inc are done.
(In reply to Katrin Fischer from comment #8) > (...) Additionally to the branchcode it would be > nice to have the currently selected language. That would allow to trigger > the visibility of text with CSS, while you have to use jQuery to do that > right now. We have already a language indicator in the html tag: lang="de-DE" This makes it possible to youse the :lang selector in your CSS. Example: body:lang(de-DE) {background-color: green;}
Ah, I didn't know that you could do that with the attribute from the html tag. Thx for the tip!
Comment on attachment 27811 [details] [review] Bug 12162 - Add class="branchcode" to body tag to make OPAC CSS-styleable per branch Proof of concept obsoleted. New patch will follow.
Created attachment 30319 [details] [review] Bug 12162 Add class="branchcode" to body tag to make OPAC CSS-styleable per branch New proof of concept, with opac-main.tt only (in bootstrap), not for signing-off. It shows how branchcode class would be inserted. If no branch is selected, body tag is: <body id="opac-main" class="branch-"> If e.g. branch FFL is selected, body tag is <body id="opac-main" class="branch-FFL "> Examples to test styling in OpacUserCSS: /*No branch, default*/ .branch- { background-color: orange; border: 10px solid green; } /*Branch FFL*/ .branch-FFL { background-color: yellow; border: 10px solid pink; } To see what files would have to be changed: Go to folder: koha-tmpl/opac-tmpl/bootstrap/en git grep "<body" Comments welcome.
I like the idea, but it seems to be a problem with translation. See coding guidelines: HTML1: Template Toolkit markup inside HTML. When can we override this rule?
Hi Marcel, Thanks for the comment. I was not aware of the translation restriction. What do you think about the following: [% IF ( LoginBranchcode ) %] <body id="opac-main" class="branch-[%LoginBranchcode%]"> [% ELSE %] <body id="opac-main" class="branch-default"> [% END %] Marc
(In reply to Marc Véron from comment #15) > Hi Marcel, > > Thanks for the comment. I was not aware of the translation restriction. > > What do you think about the following: > > [% IF ( LoginBranchcode ) %] > <body id="opac-main" class="branch-[%LoginBranchcode%]"> > [% ELSE %] > <body id="opac-main" class="branch-default"> > [% END %] > > Marc You still have the branchcode var in a html tag.
I'd run a test on this - I think it might be ok. I remember the problem happening with if-else-constructs. But we can test by updating a language and checking if translations will work ok.
The coding guidlines say: "Avoid [% IF %] inside tags because the translation tools try to read the templates as straight HTML and most TT comments inside tags probably confuse/break them." I will create a new patch for testing translation with the code from comment #15.
Created attachment 30536 [details] [review] Bug 12162 Add class="branchcode" to body tag to make OPAC CSS-styleable per branch New proof of concept, for testing translations, with if-construct outside of html tags. However, one var is inside.
Created attachment 30537 [details] [review] Bug 12162 Add class="branchcode" to body tag to make OPAC CSS-styleable per branch New proof of concept, for testing translations, with if-construct outside of html tags. However, one var is inside.
I tested translation with an install of fr-FR. With fr-FR, I get the same HTML as with en: <body id="opac-main" class="branch-FFL"> (logged in with branch FFL) and <body id="opac-main" class="branch-default"> (not logged in) The translation seems to work. I would appreciate if somebody could confirm. Marc
(In reply to Marc Véron from comment #21) > The translation seems to work. I would appreciate if somebody could confirm. I tested: [% IF ( branchFFL ) %] <body id="opac-main" class="[% branchFFL %]"> [% ELSE %] <body id="opac-main" class="default"> [% END %] And confirm that it works (tested under a 3.14.x) when translating. Although I am not sure if it is the most elegant way to address the problem?
(In reply to M. de Rooy from comment #22) > Although I am not sure if it is the most elegant way to address the problem? Hi Marcel, 'Hartstikke bedankt' for testing the translation. I'm glad that it worked. I'm aware that this solution does not look that elegant. But at the moment I have no idea how to do it better. Paxed proposed in comment #9 to work with an include similar to koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-open.inc ( BTW:I see there an if-else construct inside a html-tag? ) But the most of the .tt files have an individual ID inside the body tag, like en/modules/opac-addbybiblionumber.tt:<body id="addtolist" class="popup"> ...and it seems, that the ID can not be automatized. Maybe the IDs could be calculated from the file name, but they are not consistent with the file names. To get all of the files involved I went to koha-tmpl/opac-tmpl/bootstrap and did: git grep '<body' Any hint for an alternative to my solution is welcome :-)
(In reply to Marc Véron from comment #23) > > Paxed proposed in comment #9 to work with an include similar to > koha-tmpl/intranet-tmpl/prog/en/includes/doc-head-open.inc > ( BTW:I see there an if-else construct inside a html-tag? ) > > But the most of the .tt files have an individual ID inside the body tag, like > en/modules/opac-addbybiblionumber.tt:<body id="addtolist" class="popup"> > > Any hint for an alternative to my solution is welcome :-) Both PROCESS and INCLUDE allow for parameters, so perhaps something like [% PROCESS bodytag.inc bodyid="addtolist" %] and then use the bodyid variable as the id...
Created attachment 30606 [details] [review] Bug 12162 - Add class="branchcode" to body tag to make OPAC CSS-styleable per branch This patch is still a proof of concept for testing. I followed paxed's hints. New: Include file for the head tag. It can handle different situations (head width additional class like scrollto, head wit and without id). For the test, the include is inserted in opac-main.tt and opac-user.tt If not logged in, opac-main.pl displays: <body ID="opac-main" class="branch-default" > If logged in at branch FFL, it displays: <body ID="opac-main" class="branch-FFL" > If you log in, opac-user.pl should display <body ID="opac-user" class="branch-FFL scrollto" > Comments are welcome...
Created attachment 30607 [details] [review] Bug 12162 - Add class="branchcode" to body tag to make OPAC CSS-styleable per branch This patch is still a proof of concept for testing. I followed paxed's hints. New: Include file for the head tag. It can handle different situations (head width additional class like scrollto, head wit and without id). For the test, the include is inserted in opac-main.tt and opac-user.tt If not logged in, opac-main.pl displays: <body ID="opac-main" class="branch-default" > If logged in at branch FFL, it displays: <body ID="opac-main" class="branch-FFL" > If you log in, opac-user.pl should display <body ID="opac-user" class="branch-FFL scrollto" > Comments are welcome...
For some reason, this patch does not pass the QA tools script: FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt OK forbidden patterns OK tt_valid FAIL valid_template bodytag.inc: not found FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt OK forbidden patterns OK tt_valid FAIL valid_template bodytag.inc: not found My first idea here is that the qa tools script is still looking in prog for the include file instead of bootstrap. So it could be a false negative. Will add Jonathan to the CC list of this report.
I installed QA tools and found the following in QohA/File/Template.pm, sub check_valid_template: (...) $self->path =~ /opac-tmpl/ ? 'opac' : 'intranet'; $template_dir = File::Spec->rel2abs("koha-tmpl/${tmpl_type}-tmpl/prog/en/modules"); $include_dir = File::Spec->rel2abs("koha-tmpl/${tmpl_type}-tmpl/prog/en/includes"); (...) For OPAC, only prog is tested, not bootstrap. In this case, it is a false positive... ...and missing includes won't be failed for bootstrap.
(In reply to Marc Véron from comment #28) > I installed QA tools and found the following in QohA/File/Template.pm, sub > check_valid_template: > (...) > $self->path =~ /opac-tmpl/ > ? 'opac' > : 'intranet'; > $template_dir = > File::Spec->rel2abs("koha-tmpl/${tmpl_type}-tmpl/prog/en/modules"); > $include_dir = > File::Spec->rel2abs("koha-tmpl/${tmpl_type}-tmpl/prog/en/includes"); > (...) > > For OPAC, only prog is tested, not bootstrap. > > In this case, it is a false positive... > ...and missing includes won't be failed for bootstrap. Yeah, saw those lines too and copied Jonathan on this report.
Created attachment 30770 [details] [review] Bug 12162 - Add class="branchcode" to body tag to make OPAC CSS-styleable per branch This patch adds a branch sepecific class to all OPAC pages. Example: If not logged in, opac-main.pl displays: <body ID="opac-main" class="branch-default" > If logged in at branch FFL, it displays: <body ID="opac-main" class="branch-FFL" > If you log in, opac-user.pl should display <body ID="opac-user" class="branch-FFL scrollto" > To test: 1) Apply patch. 2) Add to syspref OPACUserCSS something highly visible, e.g. for branch FFL: .branch-FFL { background-color: yellow; border: 10px solid red; } 3) Go to OPAC and login in with a user with home branch FFL 4) Verify that colors change as appropriate. 5) Log out. Verify that colors display as before or as defined in class branch-default in OPACUserCSS 6) Display patch in patch diff view, verify that ids and classes in body tag are consistent with params bodyid and bodyclass in INCLUDE line 7) Search for regressions
(In reply to M. de Rooy from comment #27) > For some reason, this patch does not pass the QA tools script: > > FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt > OK forbidden patterns > OK tt_valid > FAIL valid_template > bodytag.inc: not found > > FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt > OK forbidden patterns > OK tt_valid > FAIL valid_template > bodytag.inc: not found > > My first idea here is that the qa tools script is still looking in prog for > the include file instead of bootstrap. So it could be a false negative. > Will add Jonathan to the CC list of this report. Fixed in the koha-qa repo: commit c2303b68be33e69198f26cd753172f995ac390eb Author: Jonathan Druart <jonathan.druart@biblibre.com> Date: Mon Aug 18 13:07:18 2014 +0200 The prog theme for the OPAC is deprecated, now use bootstrap See bug 12162 comment 27.
Thanks for updating the QA scripts, Jonathan. The patch passes now with "OK" for all files.
Created attachment 31803 [details] [review] [SIGNED OFF] Bug 12162 - Add class="branchcode" to body tag to make OPAC CSS-styleable per branch This patch adds a branch sepecific class to all OPAC pages. Example: If not logged in, opac-main.pl displays: <body ID="opac-main" class="branch-default" > If logged in at branch FFL, it displays: <body ID="opac-main" class="branch-FFL" > If you log in, opac-user.pl should display <body ID="opac-user" class="branch-FFL scrollto" > To test: 1) Apply patch. 2) Add to syspref OPACUserCSS something highly visible, e.g. for branch FFL: .branch-FFL { background-color: yellow; border: 10px solid red; } 3) Go to OPAC and login in with a user with home branch FFL 4) Verify that colors change as appropriate. 5) Log out. Verify that colors display as before or as defined in class branch-default in OPACUserCSS 6) Display patch in patch diff view, verify that ids and classes in body tag are consistent with params bodyid and bodyclass in INCLUDE line 7) Search for regressions Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected and passes QA scripts. I'm happy this patch adds a useful feature and works as tested.
Created attachment 31807 [details] [review] Bug 12162 - Add class="branchcode" to body tag to make OPAC CSS-styleable per branch This patch adds a branch sepecific class to all OPAC pages. Example: If not logged in, opac-main.pl displays: <body ID="opac-main" class="branch-default" > If logged in at branch FFL, it displays: <body ID="opac-main" class="branch-FFL" > If you log in, opac-user.pl should display <body ID="opac-user" class="branch-FFL scrollto" > To test: 1) Apply patch. 2) Add to syspref OPACUserCSS something highly visible, e.g. for branch FFL: .branch-FFL { background-color: yellow; border: 10px solid red; } 3) Go to OPAC and login in with a user with home branch FFL 4) Verify that colors change as appropriate. 5) Log out. Verify that colors display as before or as defined in class branch-default in OPACUserCSS 6) Display patch in patch diff view, verify that ids and classes in body tag are consistent with params bodyid and bodyclass in INCLUDE line 7) Search for regressions Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Patch pushed to master. Thanks Marc!