Bug 14252 - Make the OPAC language switcher available in the masthead navbar, footer, or both.
Summary: Make the OPAC language switcher available in the masthead navbar, footer, or ...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Indranil Das Gupta
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 12017
  Show dependency treegraph
 
Reported: 2015-05-21 18:26 UTC by Indranil Das Gupta
Modified: 2016-06-21 21:39 UTC (History)
11 users (show)

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


Attachments
The suggested placement at 1366x768 resolution (229.24 KB, image/jpeg)
2015-05-21 18:26 UTC, Indranil Das Gupta
Details
Bug 14252 - Move OPAC lang switcher to masthead navbar (11.06 KB, patch)
2015-05-22 21:47 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Display check on smaller devices (228.00 KB, image/jpeg)
2015-05-22 21:48 UTC, Indranil Das Gupta
Details
Sub-languages not selectable (e.g. de-DE and de-CH) (35.31 KB, image/png)
2015-05-23 09:04 UTC, Marc Véron
Details
[SIGNED-OFF] Bug 14252 - Move OPAC lang switcher to masthead navbar (11.21 KB, patch)
2015-05-23 17:37 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14252: (followup) fix lang chooser for sublanguages (5.18 MB, patch)
2015-05-23 17:37 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14252: (followup) fix lang chooser for sublanguages (4.38 KB, patch)
2015-05-23 17:41 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14252: (followup) fix lang chooser for sublanguages (4.45 KB, patch)
2015-05-23 17:50 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14252: (followup) fix lang chooser for sublanguages (5.01 KB, patch)
2015-05-23 18:02 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
View on movil (111.20 KB, image/png)
2015-05-23 18:12 UTC, Bernardo Gonzalez Kriegel
Details
Bug 14252: (followup) fix lang chooser for sublanguages (5.22 KB, patch)
2015-05-23 20:01 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
(followup) Adds OpacLangSelectorMode syspref (10.06 KB, patch)
2015-05-27 23:02 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
(followup) correction of string comparison op (2.42 KB, patch)
2015-05-28 22:55 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
[Signed-off] Bug 14252: (followup) fix lang chooser for sublanguages (5.39 KB, patch)
2015-05-31 05:00 UTC, Marc Véron
Details | Diff | Splinter Review
[Signed-off] (followup) Adds OpacLangSelectorMode syspref (10.26 KB, patch)
2015-05-31 05:02 UTC, Marc Véron
Details | Diff | Splinter Review
(Fixup) Fixes multiple issues in comment #40 (13.96 KB, patch)
2015-05-31 22:37 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
(Followup) Fixed issues pointed to in comment #40 (5.80 KB, patch)
2015-06-01 00:05 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
(fix) address the indentation for the lang loop (9.92 KB, patch)
2015-06-01 00:18 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 14252: Follow up noprint and default values (4.44 KB, patch)
2015-06-01 03:31 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14252 - Move OPAC lang switcher to masthead navbar (11.25 KB, patch)
2015-06-01 03:46 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14252: (followup) fix lang chooser for sublanguages (5.42 KB, patch)
2015-06-01 03:46 UTC, Mark Tompsett
Details | Diff | Splinter Review
(followup) Adds OpacLangSelectorMode syspref (10.29 KB, patch)
2015-06-01 03:47 UTC, Mark Tompsett
Details | Diff | Splinter Review
(Followup) Fixed issues pointed to in comment #40 (5.85 KB, patch)
2015-06-01 03:47 UTC, Mark Tompsett
Details | Diff | Splinter Review
(fix) address the indentation for the lang loop (9.98 KB, patch)
2015-06-01 03:47 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14252: Follow up noprint and default values (4.49 KB, patch)
2015-06-01 03:47 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14252: Follow up noprint and default values (4.56 KB, patch)
2015-06-01 04:14 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
(followup) addresses the QA Manager comments (5.15 KB, patch)
2015-06-01 13:54 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 14252: (followup) addresses the QA Manager comments (6.89 KB, patch)
2015-06-01 15:07 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14252 - Move OPAC lang switcher to masthead navbar (11.32 KB, patch)
2015-06-04 14:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14252: (followup) fix lang chooser for sublanguages (5.49 KB, patch)
2015-06-04 14:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14252: (followup) Adds OpacLangSelectorMode syspref (10.36 KB, patch)
2015-06-04 14:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14252: (Followup) Fixed issues pointed to in comment #40 (5.93 KB, patch)
2015-06-04 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14252: (fix) address the indentation for the lang loop (10.00 KB, patch)
2015-06-04 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14252: Follow up noprint and default values (4.63 KB, patch)
2015-06-04 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14252: (followup) addresses the QA Manager comments (6.95 KB, patch)
2015-06-04 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14252: Add sort by rfc4646_subtag in footer (1.38 KB, patch)
2015-06-04 14:07 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 Indranil Das Gupta 2015-05-21 18:26:33 UTC
Created attachment 39369 [details]
The suggested placement at 1366x768 resolution

The OPAC language switcher takes up a lot of space at the bottom of the browser. it also has issues of being not always visible (being worked on by Bernardo - http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11057).

Discussed this with @bgkriegel and @drojf on #koha. The common opinion seemed to be that this might not be a bad idea to explore, provided it doesn't blow up on handhelds.
Comment 1 Indranil Das Gupta 2015-05-22 21:47:23 UTC Comment hidden (obsolete)
Comment 2 Indranil Das Gupta 2015-05-22 21:48:59 UTC
Created attachment 39434 [details]
Display check on smaller devices

Tested on a 4.3 inch 540x960 pixel display (reference device Moto E generation 1)
Comment 3 Indranil Das Gupta 2015-05-22 21:51:10 UTC
Please particularly review the language / language grouping loop code. I'm not entirely sure if my understanding of C4/Languages.pm was up to the mark.
Comment 4 Marc Véron 2015-05-23 09:04:14 UTC
Created attachment 39436 [details]
Sub-languages not selectable (e.g. de-DE and de-CH)

I like the idea very much. However if two sublanguages are installed (e.g. de-DE and de-CH), they are no longer selectable.

I would prefer to have them on the same level (see screenshots and mockup attached).
Comment 5 Mark Tompsett 2015-05-23 12:54:15 UTC
(In reply to Marc Véron from comment #4)
> Created attachment 39436 [details]
> Sub-languages not selectable (e.g. de-DE and de-CH)
> 
> I like the idea very much. However if two sublanguages are installed (e.g.
> de-DE and de-CH), they are no longer selectable.
> 
> I would prefer to have them on the same level (see screenshots and mockup
> attached).

Would you be adverse to displaying the code (en, fr-FR, etc.) regardless of language subgroup? It makes the coding simpler. ;)
Comment 6 Indranil Das Gupta 2015-05-23 14:48:37 UTC
In reply to comment 4 and comment 5

Yes, what you say makes sense. Will be re-working the patch.
Comment 7 Bernardo Gonzalez Kriegel 2015-05-23 17:37:13 UTC Comment hidden (obsolete)
Comment 8 Bernardo Gonzalez Kriegel 2015-05-23 17:37:51 UTC Comment hidden (obsolete)
Comment 9 Bernardo Gonzalez Kriegel 2015-05-23 17:41:15 UTC Comment hidden (obsolete)
Comment 10 Bernardo Gonzalez Kriegel 2015-05-23 17:44:29 UTC
Hi, followup fixes the display of sublanguages.
First patch signed.
Comment 11 Bernardo Gonzalez Kriegel 2015-05-23 17:50:12 UTC Comment hidden (obsolete)
Comment 12 Bernardo Gonzalez Kriegel 2015-05-23 18:02:15 UTC Comment hidden (obsolete)
Comment 13 Bernardo Gonzalez Kriegel 2015-05-23 18:12:48 UTC
Created attachment 39448 [details]
View on movil

View on small device
Comment 14 Indranil Das Gupta 2015-05-23 20:01:40 UTC Comment hidden (obsolete)
Comment 15 Indranil Das Gupta 2015-05-23 20:03:21 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #12)
> Created attachment 39447 [details] [review] [review]
> Bug 14252: (followup) fix lang chooser for sublanguages
> 
> This quick fix disables sublanguages dropdown,
> show all sublanguages on same level.

Thanks Bernardo for the quickfix :-) It was just to hot to work today (RealFeel was about 50 deg C).
Comment 16 Jonathan Druart 2015-05-24 11:56:51 UTC
Indranil,
Could you please sent an email on koha or koha-devel to get opinion on this new placement?
Comment 17 Indranil Das Gupta 2015-05-24 12:37:08 UTC
(In reply to Jonathan Druart from comment #16)
> Indranil,
> Could you please sent an email on koha or koha-devel to get opinion on this
> new placement?

Jonathan,

Done! Posted a followup to my previous mail on both lists - http://lists.koha-community.org/pipermail/koha-devel/2015-May/041465.html
Comment 18 Josef Moravec 2015-05-24 18:14:48 UTC
I think it's not good idea to have the dropdown list for language switcher titled "Language". It's always going to be in the actual language and the user may not be able to recognize this as language switcher. 

If we want the dropdown, it's a bit better to call it by active language. But best approach is always to have links (or bottom bar, like in Koha) of available languages.
Comment 19 Indranil Das Gupta 2015-05-24 20:49:24 UTC
(In reply to Josef Moravec from comment #18)
> I think it's not good idea to have the dropdown list for language switcher
> titled "Language". It's always going to be in the actual language and the
> user may not be able to recognize this as language switcher. 

^^^ that is a function of the specific language's translation team working under the Koha translation project to provide. It is not a function of a patch for this bug. 

> If we want the dropdown, it's a bit better to call it by active language.

e.g. when es-ES-opac-bootstrap.po is re-generated with the entry for "Languages" in masthead.inc and translated by the Spanish L10N team, the dropdown's text will read "Idiomas" when switched to es-ES

hope that helps :-)
Comment 20 Katrin Fischer 2015-05-24 21:27:20 UTC
I think I understand Josef's point and agree.

Currently the language names are clearly visible in their own language - with this patch they would get hidden in a menu labelled in a language I might not speak or understand.

Example: If I don't know any Spanish, but end up on a Spanish website, I will look for a clear icon or better the terms 'english' or 'german' or 'Deutsch' to switch it. I won't recognize that the 'idiomas' menu is what I am looking for easily.
Comment 21 Mark Tompsett 2015-05-25 01:50:19 UTC
(In reply to Katrin Fischer from comment #20)
> I think I understand Josef's point and agree.
> 
> Currently the language names are clearly visible in their own language -
> with this patch they would get hidden in a menu labelled in a language I
> might not speak or understand.

The other thing people tend to look for are pretty little flags. ;)
That could easily be added to the patch, I think. And that would solve the "Where do I fix my language setting mix up?" problem.
Comment 22 Katrin Fischer 2015-05-25 06:50:05 UTC
Hm, I think flags are problematic in other ways, because picking one is not always easy for any language. Also - as long as it's a menu, that won't change the problem.
Comment 23 Marcel de Rooy 2015-05-25 09:07:22 UTC
I like the new position for Languages too.
But it gets kind of crowdy on that top bar (when the user is logged in and has a longer name..)
Imo Languages has more rights to be there than e.g. Search history.
Could we move Search history now to the bar with Advanced Search and Authority Search?
Could we also combine "Welcome, Marcel.."  and Logout ? Only Show the name and provide a dropdown with User options and Logout ?
Comment 24 Mirko Tietgen 2015-05-25 10:17:43 UTC
(In reply to Katrin Fischer from comment #20)
> I think I understand Josef's point and agree.
> 
> Currently the language names are clearly visible in their own language -

Unfortunately, everything is invisible right now, unless you know you have to scroll down to find the language names. It confused me more than once. And on mobile, I'd expect to find an option to switch languages in the header.

I agree with the points of Josef (visible language names) and Katrin (no flags). 

We could hide the language names behind a universal "language switcher" symbol in the header, but there is none. Things I have seen used were a globe symbol, a flag symbol (unspecified), Fontawesome has this http://fontawesome.io/icon/language/ and there are probably more. Showing the language code of the language in use is also a way of doing things, but you need people to be able to read the script in use and recognize it as something representing a language.

I found this to be an interesting read on the topic. http://flagsarenotlanguages.com/blog/2013/10/case-study-onefinestay-com-and-dropdown-language-selection/

Having thought about it a little more, I'd say moving the switcher is the wrong way to go. A switcher in the header should be an addition, not a replacement for the one in the footer. And the one in the footer should be visible without any scrolling, as it used to be, at least on a desktop size screen.
Comment 25 Mark Tompsett 2015-05-26 05:33:59 UTC
(In reply to Mirko Tietgen from comment #24)
> I found this to be an interesting read on the topic.
> http://flagsarenotlanguages.com/blog/2013/10/case-study-onefinestay-com-and-
> dropdown-language-selection/

Interesting read. I think this is very much applicable to our scenario here.


> Having thought about it a little more, I'd say moving the switcher is the
> wrong way to go. A switcher in the header should be an addition, not a
> replacement for the one in the footer.

I'll agree, because some people are already used to the footer language switcher.


> And the one in the footer should be
> visible without any scrolling, as it used to be, at least on a desktop size
> screen.

Isn't that what 11057 is about?
Comment 26 Marc Véron 2015-05-26 09:16:54 UTC
(In reply to Katrin Fischer from comment #22)
> Hm, I think flags are problematic in other ways, because picking one is not
> always easy for any language.  (...)

E.g. for Switzerland: We have German, French, Italian languages and it always looks a bit odd to see them symbolized by "foreign" flags (Germany, France, Italy)... 

(In reply to  Mirko Tietgen from comment #24)
> A switcher in the header should be an addition, not a replacement for the one
> in the footer. (...)

Why not having a syspref (Language selection in header or footer with footer as default)?
Comment 27 Jonathan Druart 2015-05-26 10:28:20 UTC
Set to In Discussion.
Comment 28 Indranil Das Gupta 2015-05-26 11:33:58 UTC
Hi Marc

(In reply to Marc Véron from comment #26)
> (In reply to Katrin Fischer from comment #22)
> > Hm, I think flags are problematic in other ways, because picking one is not
> > always easy for any language.  (...)
> 
> E.g. for Switzerland: We have German, French, Italian languages and it
> always looks a bit odd to see them symbolized by "foreign" flags (Germany,
> France, Italy)... 

The Swiss have 3, well think about India, 22 officially recognised languages, all well-represented in Unicode. :)

As for the flag bit, IMHO the better way to go about it is to use the flag of the country the language is representing e.g. pt-PT should use Portugal's flag, whereas pt-BR should use Brazil's flag. That way all 3 languages of Switzerland should be tagged against the Swiss flag.

> (In reply to  Mirko Tietgen from comment #24)
> > A switcher in the header should be an addition, not a replacement for the one
> > in the footer. (...)
> 
> Why not having a syspref (Language selection in header or footer with footer
> as default)?

+1
Comment 29 Mirko Tietgen 2015-05-27 08:02:50 UTC
(In reply to Marc Véron from comment #26)

> Why not having a syspref (Language selection in header or footer with footer
> as default)?

Because I would like both. :P So a third option (»both«) would be nice.
Comment 30 Marc Véron 2015-05-27 11:33:18 UTC
(In reply to Mirko Tietgen from comment #29)
> (In reply to Marc Véron from comment #26)
> 
> > Why not having a syspref (Language selection in header or footer with footer
> > as default)?
> 
> Because I would like both. :P So a third option (»both«) would be nice.

Yes, indeed :)
Comment 31 Viktor Sarge 2015-05-27 12:06:47 UTC
Interesting. Both have their merits. What I like about the current solution is that it's visible (if we fix that bug...) and that switching language is a one off thing that you don't need to waste screen space in the top on mobile devices for. 

Using a drop down seems reasonable when you have many languages, but it does waste screen space on the top of pages (having to scroll extra on every page in mobile because of a one off language switcher is no fun). Also it's hard to make it obvious that it's there. 

Perhaps a hybrid with a drop down showing when there's too many languages installed to display the all? I still vote for the footer however. 

For a drop down I would solve identifying it with using the language names in their own alphabet. If I see a drop down on an english web that displays arabic my first thought it that it will also have french, german, hungarian and swedish (etcetera) :)
Comment 32 Viktor Sarge 2015-05-27 12:27:49 UTC
Oh... please disregard the note about scrolling past language chooser on every page of mobile device. That was me being to quick when looking at the screenshots :) 

But if I (now finally) understand the issue correctly we would indeed need a way to better communicate where you choose the language.
Comment 33 Gaetan Boisson 2015-05-27 13:42:50 UTC
Interesting topic!

My 2 cents:

- I like the icon used by Indranil in his first suggestions (a very generic flag), i think a lot of people will associate a flag with a language selector on a website. There are other interesting options using language characters, but i have the feeling they need more space to be readable.
- Flags as languages sound like a bad idea, France is not the French language and so on, but it seems we all agree on this.
- They could be used for sub-languages next to the language name itself, it works for de-DE and de-CH for instance. But i am not 100% sure it works everywhere, so we could still end up in political territory. (If we had an Euskara translation for instance, a language spoken in the southwest of France and a part of Spain, where a number of claims for independence have been made. Choosing one flag over another will be a political choice...)
- I think we should keep links at the bottom, things at the bottom don't waste space, and some users will find them there.

All in all i think we are very close to it: a language picker at the top, with a generic flag icon, sub-languages listed one after another, and links at the bottom where they used to be.

I am not even sure we need another syspref, if we have a css id for the top picker unhappy people will be able to hide it easily.
Comment 34 Mark Tompsett 2015-05-27 15:40:16 UTC
(In reply to Gaetan Boisson from comment #33)
> - I like the icon used by Indranil in his first suggestions (a very generic
> flag), i think a lot of people will associate a flag with a language
> selector on a website. There are other interesting options using language
> characters, but i have the feeling they need more space to be readable.

Yes, when I said flag, I didn't mean it had to be a country flag. A generic language selector icon of some sort (whether a flag, globe, etc.).


> [SNIP] Choosing one flag over another will be a political choice...)

Hence, a generic icon of some sort. Flag shaped? Perhaps. Globe. Whatever, some graphic that screams, "Hey, this is where the language can be changed."


> - I think we should keep links at the bottom, things at the bottom don't
> waste space, and some users will find them there.

Some users will not scroll below the current screen visual. :(


> All in all i think we are very close to it: a language picker at the top,
> with a generic flag icon, sub-languages listed one after another, and links
> at the bottom where they used to be.

Generally, perhaps. Yes.


> I am not even sure we need another syspref, if we have a css id for the top
> picker unhappy people will be able to hide it easily.

I always prefer a system preference option over a CSS option. The former requires less technical expertise. Someone playing around with Koha for evaluation purposes is not necessarily going to have CSS expertise or such available to them. Don't force librarians to learn HTML, CSS, Perl, Linux, etc. in order to use Koha. Let's keep the technical level required for customizations low if possible.
Comment 35 Indranil Das Gupta 2015-05-27 23:02:23 UTC Comment hidden (obsolete)
Comment 36 Indranil Das Gupta 2015-05-28 22:55:44 UTC Comment hidden (obsolete)
Comment 37 Mark Tompsett 2015-05-28 23:31:08 UTC
Comment on attachment 39650 [details] [review]
(followup) correction of string comparison op

http://template-toolkit.org/docs/manual/Directives.html#section_IF_UNLESS_ELSIF_ELSE

Don't confuse Template Toolkit and Perl comparisons. It's easy to do.
Comment 38 Marc Véron 2015-05-31 05:00:48 UTC Comment hidden (obsolete)
Comment 39 Marc Véron 2015-05-31 05:02:41 UTC Comment hidden (obsolete)
Comment 40 Mark Tompsett 2015-05-31 17:11:09 UTC
Comment on attachment 39707 [details] [review]
[Signed-off] (followup) Adds OpacLangSelectorMode syspref

Review of attachment 39707 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
@@ +321,5 @@
> +            - "Display language selector on "
> +            - pref: OpacLangSelectorMode
> +              choices:
> +                  both: "both masthead and footer"
> +                  head: "only header"

Choose 'masthead' or 'header' and be consistent.

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/masthead.inc
@@ +71,4 @@
>                          <a id="user-menu-trigger" class="pull-right" href="#"><i class="icon-user"></i> <span class="caret"></span></a>
>                          <div id="members">
>                              <ul class="nav pull-right">
> +                            [% IF ( Koha.Preference( 'OpacLangSelectorMode' ) == 'both' ) || ( Koha.Preference( 'OpacLangSelectorMode' ) == 'head' ) %]

I know it is an extra line of code, but the whole Koha.Preference() call is 'slow', so a double call is 2n. Variable access in Template Toolkit is most certainly faster than a call.
[% SET OpacLangSelectorMode = Koha.Preference('OpacLangSelectorMode') %]
[% IF OpacLangSelectorMode == 'both' || OpacLangSelectorMode == 'head' %]
is likely faster (n+k, where 1<k<n).

@@ +106,4 @@
>                                      [% END # / UNLESS ( one_language_enabled ) %]
>                                  [% END # / IF ( languages_loop && opaclanguagesdisplay ) %]
>                              [% END # / IF opaclanguagesdisplay %]
> +                            [% END # / IF OpacLangSelectorMode %]

Both sections are new code, could we get the indentation right?

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-bottom.inc
@@ +70,5 @@
> +                </div> <!-- / #changelanguage -->
> +            [% END # / UNLESS ( one_language_enabled ) %]
> +        [% END # / IF ( languages_loop && opaclanguagesdisplay ) %]
> +    [% END # / IF opaclanguagesdisplay %]
> +[% END %]

Why not indent this properly?
Comment 41 Katrin Fischer 2015-05-31 20:15:33 UTC
Looking at Mark's comments I think at least the first 2 should be fixed - can you please take a look? Thx!
Comment 42 Indranil Das Gupta 2015-05-31 22:37:54 UTC Comment hidden (obsolete)
Comment 43 Indranil Das Gupta 2015-06-01 00:05:32 UTC Comment hidden (obsolete)
Comment 44 Indranil Das Gupta 2015-06-01 00:18:08 UTC Comment hidden (obsolete)
Comment 45 Mark Tompsett 2015-06-01 01:37:17 UTC
Comment on attachment 39707 [details] [review]
[Signed-off] (followup) Adds OpacLangSelectorMode syspref

Review of attachment 39707 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-bottom.inc
@@ +32,5 @@
> +[% IF ( Koha.Preference( 'OpacLangSelectorMode' ) == 'both' ) || ( Koha.Preference( 'OpacLangSelectorMode' ) == 'foot' ) %]
> +    [% IF ( opaclanguagesdisplay ) %]
> +        [% IF ( languages_loop && opaclanguagesdisplay ) %]
> +            [% UNLESS ( one_language_enabled ) %]
> +                <div id="changelanguage" class="navbar navbar-fixed-bottom navbar-static-bottom">

Why is 'noprint' missing from the class list?
Comment 46 Mark Tompsett 2015-06-01 02:23:00 UTC
Comment on attachment 39725 [details] [review]
(Followup) Fixed issues pointed to in comment #40

Review of attachment 39725 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/atomicupdate/bug_14252-OpacLangSelectorMode_syspref.sql
@@ +1,4 @@
>  --
>  -- Adds OpacLangSelectorMode syspref for bug 14252
>  --
> +INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('OpacLangSelectorMode','both','both|mast|foot','Select the location to display the language selector','Choice');

The default value should be 'foot'. New system preferences should not change current behaviours.

::: installer/data/mysql/sysprefs.sql
@@ +270,4 @@
>  ('OpacItemLocation','callnum','callnum|ccode|location','Show the shelving location of items in the opac','Choice'),
>  ('OPACItemsResultsDisplay','0','','If OFF : show only the status of items in result list.If ON : show full location of items (branch+location+callnumber) as in staff interface','YesNo'),
>  ('OpacKohaUrl','1',NULL,'Show \'Powered by Koha\' text on OPAC footer.',NULL),
> +('OpacLangSelectorMode','both','both|mast|foot','Select the location to display the language selector','Choice'),

'foot', not 'both'.
Comment 47 Indranil Das Gupta 2015-06-01 02:38:35 UTC
(In reply to M. Tompsett from comment #46)
> Comment on attachment 39725 [details] [review] [review]
> (Followup) Fixed issues pointed to in comment #40
> 
> Review of attachment 39725 [details] [review] [review]:
> -----------------------------------------------------------------
<snipped>
 
> The default value should be 'foot'. New system preferences should not change
> current behaviours.

Mark,

While I acknowledge the part that it shouldn't change current behavior, I do not agree with you on this. Earlier there was only 'foot', it is still retained by 'both', the masthead is an addition, not a replacement. The whole point of having the masthead switcher is nullified IMO if it is not available along with the earlier footer placement.
Comment 48 Mark Tompsett 2015-06-01 03:31:06 UTC Comment hidden (obsolete)
Comment 49 Mark Tompsett 2015-06-01 03:46:26 UTC Comment hidden (obsolete)
Comment 50 Mark Tompsett 2015-06-01 03:46:52 UTC Comment hidden (obsolete)
Comment 51 Mark Tompsett 2015-06-01 03:47:12 UTC Comment hidden (obsolete)
Comment 52 Mark Tompsett 2015-06-01 03:47:33 UTC Comment hidden (obsolete)
Comment 53 Mark Tompsett 2015-06-01 03:47:42 UTC Comment hidden (obsolete)
Comment 54 Mark Tompsett 2015-06-01 03:47:58 UTC Comment hidden (obsolete)
Comment 55 Indranil Das Gupta 2015-06-01 04:14:57 UTC Comment hidden (obsolete)
Comment 56 Katrin Fischer 2015-06-01 05:45:29 UTC
Hm, I think maybe a compromise: new installations (sysprefs.sql) could be set to both, while we keep it for existing installations at 'foot'. 
Looking at this, and hoping not to be to nit-picky - why not use top|both|footer? Mast looks a bit weird and quite template specific.
Comment 57 Indranil Das Gupta 2015-06-01 13:54:52 UTC Comment hidden (obsolete)
Comment 58 Indranil Das Gupta 2015-06-01 13:57:19 UTC
(In reply to Katrin Fischer from comment #56)
> Hm, I think maybe a compromise: new installations (sysprefs.sql) could be
> set to both, while we keep it for existing installations at 'foot'. 

Mark and I both agree with that! Nice suggestion!

> Looking at this, and hoping not to be to nit-picky - why not use
> top|both|footer? Mast looks a bit weird and quite template specific.

^^^ Done!
Comment 59 Mark Tompsett 2015-06-01 15:07:45 UTC Comment hidden (obsolete)
Comment 60 Jonathan Druart 2015-06-04 14:06:49 UTC
Created attachment 39857 [details] [review]
Bug 14252 - Move OPAC lang switcher to masthead navbar

The OPAC language switcher takes up a lot of space at the bottom of the
browser. It also has issues of being not always visible.
This patch adds the switcher to the masthead navbar and removes from the
opac-bottom.inc navbar.

Test plan
=========

1/ Apply patch
2/ Set opaclanguagesdisplay syspref to 'show'.
3/ Add a few languages i.e. es-ES, fr-FR and de-DE
     $ cd misc/translator
     $ perl translate <langcode>
4/ Enable installed languages for OPAC use by checking on 'opaclanguages'
   under I18N/L10N sysprefs
5/ Go to the OPAC, you should have a "Languages" dropdown on the masthead
   navbar, the opac-bottom.inc navbar should no longer be there.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Works, no koha-qa errors.
Followup fix language chooser for sublanguages

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 61 Jonathan Druart 2015-06-04 14:06:54 UTC
Created attachment 39858 [details] [review]
Bug 14252: (followup) fix lang chooser for sublanguages

This quick fix disables sublanguages dropdown,
show all sublanguages on same level.

To test:
1) Apply both patches
2) Translate to have sublanguages (en-NZ, de-DE, de-CH)
3) Go to opac, confirm you can select any lang/sublang
4) Check on movil device

4th version.
Removed rfc4646 subtag on sublanguages, think it's cleaner.
Currently on master all translated languages/regions have
valid description ('en' at least)
Added language sort using rfc4646 subtag

Signed-off-by: Indranil Das Gupta <indradg@gmail.com>
Followup fixes the issue with lang group dropdowns. Works well
across standard desktop as well as mobile device with small screens
(checked on 4.3" / 4.7" / 5")

Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 62 Jonathan Druart 2015-06-04 14:06:59 UTC
Created attachment 39859 [details] [review]
Bug 14252: (followup) Adds OpacLangSelectorMode syspref

Based on discussion, this followup does the following:

1/ brings back the switcher on opac-bottom.inc
2/ adds a syspref OpacLangSelectorMode to toggle between the three
   modes:
     (a) show switcher both on masthead and footer (default)
     (b) footer only
     (b) masthead only

Test plan
=========

1/ apply earlier patches attached to this bug in their correct order
2/ apply this followup patch
3/ run updatedatabase.pl to add in the atomic update
4/ goto admin/preferences.pl?tab=opac
5/ look up OpacLangSelectorMode, it should be set with default value
   "both masthead and footer"
6/ check OPAC to see if both locations show the selectors
7/ change OpacLangSelectorMode to 'only header' and 'only footer' at
   each iteration, and check if the selection has correctly toggled
   the selectors. It should

Note: make sure you do not have the patch 11057 applied on the branch
      from before, otherwise merge conflict might happen.

http://bugs.koha-community.org/show_bug.cgi?id=14252

Followed test plan. Works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 63 Jonathan Druart 2015-06-04 14:07:04 UTC
Created attachment 39860 [details] [review]
Bug 14252: (Followup) Fixed issues pointed to in comment #40

Based on Mark's inputs in comment #40 -

[1] selected 'mast' (short of 'masthead') and updated :
   (a) bug_14252-OpacLangSelectorMode_syspref.sql
   (b) sysprefs.sql
   (c) opac.pref
[2] Reduced the calls to Koha.Preference() from :
   (a) masthead.inc
   (b) opac-bottom.inc

After applying this patch, update the database once for it to pick
up the new option values -'both|mast|foot'.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 64 Jonathan Druart 2015-06-04 14:07:09 UTC
Created attachment 39861 [details] [review]
Bug 14252: (fix) address the indentation for the lang loop

tidy the indentation for that lang selector loop code.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 65 Jonathan Druart 2015-06-04 14:07:13 UTC
Created attachment 39862 [details] [review]
Bug 14252: Follow up noprint and default values

TEST PLAN
---------
0) Back up your DB
1) open OPAC
   -- should only have languages in footer.
      Annoyingly below the fold.
2) Drop your koha database, and create a blank one.
3) Apply all patches
4) Go to staff client and do an install of koha, with
   all the dummy data.
5) Once installed, refresh your OPAC page
   -- language selector position should be in footer
      still as expected.
   -- this confirms the sysprefs.sql change.
6) Restore your DB
7) run the updatedatabase.pl script
8) Refresh the OPAC page
   -- should still only have languages in the footer.
   -- this confirms that upgrade won't change anything.
   -- this confirms the atomic update.
9) git diff origin/master
   -- the opac-bottom.inc difference should only
      have changes around a SET, IF, and END.
      This confirms the noprint was fixed.
10) In the staff client, change the OPAC system preference,
    OpacLangSelectorMode, to all three values.
    For each value, refresh the OPAC page, and confirm the
    position of the language selector.
    -- should work as expected.
11) run koha qa test tools

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 66 Jonathan Druart 2015-06-04 14:07:18 UTC
Created attachment 39863 [details] [review]
Bug 14252: (followup) addresses the QA Manager comments

Addresses Katrin's comments in comment# 56

TEST PLAN
---------
 1) Back up your DB
 2) Ensure you have multiple languages, including
    some that have sub-languages (e.g. de-DE, de-CH)
    -- cd misc/translator
    -- perl translate install {language code}
 3) Ensure that you have all the languages enabled
    -- Staff client -> Home -> Global system preferences
       --> I18N/L10N
       Check all the languages in opaclanguages.
       Ensure that opaclanguagesdisplay is 'Allow'
 4) Open OPAC
    -- should only have languages in footer.
       Annoyingly below the fold.
 5) Drop your koha database, and create a blank one.
 6) Apply all patches
 7) Reinstall all the known languages, so the templates
    are appropriately updated.
 8) Go to staff client and do an install of koha, with
    all the dummy data.
 9) Ensure you have all the languages enabled (see (3))
10) Refresh your OPAC page
    -- language selector position should be in both
       the footer and at the top as expected.
    -- this confirms the sysprefs.sql change.
11) Restore your DB
12) run the updatedatabase.pl script
13) Ensure you have all the languages enabled (see (3))
14) Refresh the OPAC page
    -- should still only have languages in the footer.
    -- this confirms that upgrade won't change anything.
    -- this confirms the atomic update.
15) git diff origin/master
    -- the opac-bottom.inc difference should only
       have changes around a SET, IF, and END.
       This confirms the noprint was fixed.
16) In the staff client, change the OPAC system preference,
    OpacLangSelectorMode, to all three values.
    For each value, refresh the OPAC page, and confirm the
    position of the language selector.
    -- should work as expected.
17) run koha qa test tools

NOTE: Tested as a single batch together.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 67 Jonathan Druart 2015-06-04 14:07:22 UTC
Created attachment 39864 [details] [review]
Bug 14252: Add sort by rfc4646_subtag in footer

This patch sort by rfc4646_subtag the languages in the footer.
Same as in the header.

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 68 Tomás Cohen Arazi 2015-06-24 14:33:46 UTC
Enhancement pushed to master.

Thanks Indranil!