Bug 36337 - Hiding lists with OpacPublic breaks styling for language list
Summary: Hiding lists with OpacPublic breaks styling for language list
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on: 33244
Blocks:
  Show dependency treegraph
 
Reported: 2024-03-17 16:59 UTC by Katrin Fischer
Modified: 2024-06-27 01:46 UTC (History)
2 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
Bug 36337: Hiding lists with OpacPublic breaks styling for language list (2.72 KB, patch)
2024-06-26 12:03 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 36337: Hiding lists with OpacPublic breaks styling for language list (2.76 KB, patch)
2024-06-27 01:40 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2024-03-17 16:59:41 UTC
Added from the comment on bug 33244:

Hello, [this patch] seems to have a side effect of the "Language" dropdown now being awkwardly placed on center-right (regardless of whether user is logged in our out). This is caused by the below CSS rules whose blame traces back to this patch, and removing them fixes the issue.

#members {
+    flex-grow: 1;
+     justify-content: flex-end;

Is this intentional change? I can only assume it's not, since I don't see any mention of this in the discussion.

Screenshots of the problem: https://imgur.com/a/hDZvn7B

It looks like this patch conditionally hides cart+lists. The CSS above then makes only language shown on the left, and the login on the right. But when things aren't hidden, it changes the original position.

Should I create a new bug about this?
Comment 1 Michał 2024-03-18 08:22:33 UTC
Thanks for creating the bug.

To clarify though: the issue shows up when the lists are NOT hidden (which is the default). They added the feature of hiding lists and fixed the styling when they're hidden, but in turn broke the styling when they're normally shown.
Comment 2 Michał 2024-03-19 07:23:19 UTC
So I see three possible ways of fixing it:

1. Somehow making it so that while lists are shown, the languages are on the right as they were, and if hidden, on the left (will probably require adding a new CSS class to toggle the behavior, as I couldn't figure out any CSS trick for this)

2. Reverting and making it so that languages are always on the right, while nothing is shown on the left if lists are disabled

3. Make languages always on the left

I wouldn't favor option #3 since that changes the UX and I think language belongs to the right next to user settings. In spirit of the original bug, I think they intended for #1 (to avoid keeping the space on left empty?). I personally think #2 might be the best? I mean there are many sites that have logo on the left and all toolbars on the right, it would look perfectly okay.

I could try taking a stab at making a patch, but I'd welcome some opinions on the above I guess...
Comment 3 Owen Leonard 2024-06-26 12:03:19 UTC
Created attachment 168139 [details] [review]
Bug 36337: Hiding lists with OpacPublic breaks styling for language list

This patch adjusts the CSS of elements in the OPAC header so that menu
items are correctly aligned with various combinations of the OpacPublic
and OpacLangSelectorMode system preferences.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client)

- View the header menu in the OPAC with these combinations of settings:
  - OpacPublic = On; OpacLangSelectorMode = Top;
  - OpacPublic = On; OpacLangSelectorMode = Footer;
  - OpacPublic = Off; OpacLangSelectorMode = Top;
  - OpacPublic = Off; OpacLangSelectorMode = Footer;
- With each configuration, test that the elements in the header (Logo,
  Cart/List menu, Language selctor, and user menu items) align
  correctly. The Cart/List menu items should always align to the left,
  the Language selector and user menu should aways align to the right.

Sponsored-by: Athens County Public Libraries
Comment 4 David Nind 2024-06-27 01:40:01 UTC
Created attachment 168186 [details] [review]
Bug 36337: Hiding lists with OpacPublic breaks styling for language list

This patch adjusts the CSS of elements in the OPAC header so that menu
items are correctly aligned with various combinations of the OpacPublic
and OpacLangSelectorMode system preferences.

To test, apply the patch and rebuild the OPAC CSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client)

- View the header menu in the OPAC with these combinations of settings:
  - OpacPublic = On; OpacLangSelectorMode = Top;
  - OpacPublic = On; OpacLangSelectorMode = Footer;
  - OpacPublic = Off; OpacLangSelectorMode = Top;
  - OpacPublic = Off; OpacLangSelectorMode = Footer;
- With each configuration, test that the elements in the header (Logo,
  Cart/List menu, Language selctor, and user menu items) align
  correctly. The Cart/List menu items should always align to the left,
  the Language selector and user menu should aways align to the right.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2024-06-27 01:46:17 UTC
Testing notes (using KTD):

1. Need to have another language installed - see https://gitlab.com/koha-community/koha-testing-docker#translation-files
   . cd misc/translator/po
   . git fetch origin
   . git reset --hard origin/main
   . in the koha shell (ktd --shell), gulp po:update --lang fr-CA
   . exit from shell, and change to a root shell: ktd --root --shell
   . cd koha
   . koha-translate --install fr-CA --dev kohadev
   . restart_all

2. Enable the language for the OPAC:
   . OPACLanguages system preference - make sure languages are ticked
   . opaclanguagesdisplay system preference set to Allow

3. Regenerate the CSS: yarn build

4. Restart everything: restart_all