Bug 20168 - Update of the OPAC bootstrap template to bootstrap v4
Summary: Update of the OPAC bootstrap template to bootstrap v4
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Jonathan Druart
URL: https://gitlab.com/koha-dev/koha-dev/...
Keywords: release-notes-needed
: 18031 (view as bug list)
Depends on: 14862
Blocks: 25771 25775 26164 26419 26478 26505 26512 26519 26735 26749 26752 26758 26810 26465 26720
  Show dependency treegraph
 
Reported: 2018-02-09 12:34 UTC by Elias
Modified: 2020-10-24 00:37 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Large patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Elias 2018-02-09 12:34:53 UTC Comment hidden (obsolete)
Comment 1 Owen Leonard 2018-02-09 13:48:57 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2018-02-09 13:58:17 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2018-02-10 22:12:18 UTC Comment hidden (obsolete)
Comment 4 Owen Leonard 2018-06-29 14:02:03 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2018-06-29 14:02:38 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2020-06-03 17:37:20 UTC
Work in progress:

https://gitlab.com/koha-dev/koha-dev/-/tree/wip-2020-05-14-opac-bootstrap-4

If you test, you'll need to run 'yarn install' before running 'yarn build --view opac'
Comment 7 Katrin Fischer 2020-06-07 13:33:51 UTC
Hi Owen, is this ready for testing or are you still working on it?
Comment 8 Owen Leonard 2020-06-08 18:49:53 UTC
Remote branch:


https://gitlab.com/koha-dev/koha-dev/-/tree/wip-2020-05-14-opac-bootstrap-4

Information and test plan:

    This patch updates the version of Bootstrap in the OPAC from 2.3.1 to
    4.5.0. The Bootstrap JavaScript files have been replaced with custom
    builds of the 4.5.0 JavaScript source files. The Bootstrap CSS is now
    built into the OPAC CSS by loading the required Bootstrap 4.5.0 SCSS
    files in node_modules.

    OPAC SCSS now starts with Bootstrap customizations:

    /* Bootstrap variable customizations */
    $headings-color: #727272;
    ...

    Followed by loading the necessary Bootstrap SCSS files:

    /* Bootstrap imports */
    @import "../../../../../node_modules/bootstrap/scss/functions";
    @import "../../../../../node_modules/bootstrap/scss/variables";
    ...

    Followed by our CSS. The build process for generating compiled CSS now
    creates a file which bundles Bootstrap CSS and ours. Removed from the
    Koha source: Bootstrap CSS files, Bootstrap "glyphicons" images.

    The upgrade to Bootstrap 4 involved a lot of markup changes to conform
    with new Bootstrap classes, especially in classes related to the grid.
    Besides duplicating the grid we used before, this upgrade adds some new
    features made possible by Bootstrap 4.5's use of flexbox as a layout
    tool. This includes custom ordering of columns based on class names:
    https://getbootstrap.com/docs/4.5/layout/grid/#order-classes.

    Other areas where the most changes have been made: Navigation menus,
    breadcrumb menus, buttons, dropdowns.

    Unrelated changes: Indentation corrections, removal of invalid
    "//<![CDATA[" markers, removal of invalid script type attributes.

    To test, apply the patch and run 'yarn install' to install Bootstrap as
    an npm module. Run 'yarn build --view opac' to regenerate the OPAC CSS.

    Test as many aspect of the OPAC as possible, viewing pages at various
    browser widths to confirm that everything adjusts well. Test with
    various OPAC interface system preferences enabled and disabled.

    Test self checkout and self checkin.

    Known issues: RTL support has not been updated.
Comment 9 Owen Leonard 2020-07-02 20:58:54 UTC
Remote branch has been rebased on master, including:

Bug 18911, Option to set preferred language in OPAC
Bug 22672, Replace <i> tags with <em> AND <b> tags with <strong> in the OPAC
Bug 22807, Accessibility: Add 'Skip to main content' link
Bug 24156, Basket - Make sort order and number of items to display configurable
Bug 25151, Accessibility: The 'Your cart' page does not contain a level-one header
Bug 25154, Accessibility: The 'Search results' page does not use heading markup where content is introduced
Bug 25238, Accessibility: Multiple 'H1' headings exist in the full record display

Some QA tool warnings have been fixed. There seem to be several false positives too.
Comment 10 Lucas Gass 2020-07-29 17:39:03 UTC
Initial impressions:

In the navbar when there are no added lists and the user is NOT logged in the text breaks ‘Log in to create your own lists’. (https://snipboard.io/YZdXTv.jpg) This seems to look fine once you have a list, even if the list name is long: (https://snipboard.io/9CAMFl.jpg)

On the item details page the More Searches dropdown (inside #ulactioncontainer) is not a dropdown. Maybe its missing a div container with the dropdown-menu class? (https://snipboard.io/jcB4Mf.jpg)

There are many inputs that get a Koha green border when focused that now also get the Bootstrap blue border, they look odd when paired: (https://snipboard.io/6KajBs.jpg)

When you view your cart (basket ) the remove, add to list, place hold buttons are blue when disabled and blue when enabled so its hard to tell the difference. They used to be grey when disabled. (https://snipboard.io/uE7zwb.jpg)
Comment 11 Owen Leonard 2020-07-31 13:14:24 UTC
I've rebased and updated the remote branch with some fixes:

     - Fixed markup of navbar item "Log in to create your own lists"
     - Improved consistency of input and link focus borders (needs
       more improvement).
     - Corrected style of disabled link buttons in the Cart.

(In reply to Lucas Gass from comment #10)

> On the item details page the More Searches dropdown (inside
> #ulactioncontainer) is not a dropdown.

This is a problem because the Bootstrap dropdown markup changed in version 4. I'm still puzzling over this... There is probably a CSS solution, preferable to trying to add a db update which tries to correct existing values in the system preference.

Currently for it to work properly you have to update the  OPACSearchForTitleIn preference with this text, removing the <li>'s and adding a "dropdown-item" class to the links:


<a class="dropdown-item" href="https://worldcat.org/search?q={TITLE}" target="_blank">Other Libraries (WorldCat)</a>
<a class="dropdown-item" href="https://scholar.google.com/scholar?q={TITLE}" target="_blank">Other Databases (Google Scholar)</a>
<a class="dropdown-item" href="https://www.bookfinder.com/search/?author={AUTHOR}&amp;title={TITLE}&amp;st=xl&amp;ac=qr" target="_blank">Online Stores (Bookfinder.com)</a>
<a class="dropdown-item" href="https://openlibrary.org/search?author=({AUTHOR})&title=({TITLE})" target="_blank">Open Library (openlibrary.org)</a>
Comment 12 Martin Renvoize 2020-08-05 14:52:28 UTC
Generally I'm liking it so far...

* The search bar doesn't react properly when shrinking and growing the window width however.
* I'm not sure about the increase in whitespace and sizes.. the login modal looks huge and the space from the top of the page to the start of content seems to have increased considerable with various increases in padding for each element?
Comment 13 Owen Leonard 2020-08-06 19:01:08 UTC
(In reply to Martin Renvoize from comment #12)

> * The search bar doesn't react properly when shrinking and growing the
> window width however.

This has been fixed in the remote branch.

> * I'm not sure about the increase in whitespace and sizes...

I'm happy to reevaluate this if others are bothered by it. I think it might be a difference in Bootstrap defaults.
Comment 14 Martin Renvoize 2020-08-12 14:17:23 UTC
OK.. Well.. I ran this for the better part of a day and worked through most pages.. I'm happy that I didn't find any real bugs..

Please add my SO line to the patches on the branch :)
Comment 15 Katrin Fischer 2020-08-23 00:14:19 UTC
I've tried testing this from the branch, but there is an error on the yarn build:

$ yarn build --view opac
yarn run v1.22.4
$ node_modules/.bin/gulp build --view opac
[10:47:07] Using gulpfile ~/kohaclone/gulpfile.js
[10:47:07] Starting 'build'...
Error in plugin 'sass'
Message:
    koha-tmpl/opac-tmpl/bootstrap/css/src/_common.scss
Error: File to import not found or unreadable: ../../../../../node_modules/bootstrap/scss/functions.
        on line 39 of koha-tmpl/opac-tmpl/bootstrap/css/src/_common.scss
        from line 1 of koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss
>> @import "../../../../../node_modules/bootstrap/scss/functions";

   ^
Comment 16 Jonathan Druart 2020-08-24 11:54:30 UTC
yarn install
yarn build
yarn build --view=opac

all works ok for me on KTD
Comment 17 Tomás Cohen Arazi 2020-08-26 13:27:07 UTC
I've submitted to a branch on our gitlab a follow-up that makes the gulpfile and package.json back to master's, and also sets bootstrap version to 4.5.2. Regardless, there's a problem on search results paging, which Owen is already working on. Setting to ASSIGNED in the meantime.
Comment 18 Owen Leonard 2020-08-31 15:22:06 UTC
I've rebased and made corrections on the remote branch:


https://gitlab.com/koha-dev/koha-dev/-/tree/wip-2020-05-14-opac-bootstrap-4
Comment 19 Jonathan Druart 2020-09-03 08:56:16 UTC
There are merge marker conflicts left in the last patch

koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt:<<<<<<< HEAD
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt:<<<<<<< HEAD
Comment 20 Jonathan Druart 2020-09-03 08:57:40 UTC
I think the patches should be squashed. The first patch contain a lot of merge marker conflicts, that will make further conflicts a nightmare to resolve.
Comment 21 Jonathan Druart 2020-09-03 09:07:38 UTC
Also can you confirm the following 3 errors are not valid?

 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
   FAIL   forbidden patterns                                                                                                                                                                                                       forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 1174)                                                                                                         forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 1180)    
                                                                                                                                                                                                                                                                                                                                                                                                                                                  
 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt                                                                                                                                                   
   FAIL   forbidden patterns                                                                                                                                                                                       
                forbidden pattern: Do *not* include full path to js files, use the Asset TT plugin (bug 20538) (line 625)                                                                                          

 FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
   FAIL   forbidden patterns
                forbidden pattern: Font Awesome icons need the aria-hidden attribute for accessibility purpose (see bug 25166)  (line 124)
Comment 22 Jonathan Druart 2020-09-03 09:28:53 UTC
1. https://snipboard.io/9kNAL8.jpg
Don't you think the login inputs are too wide?

2. https://snipboard.io/TlIkcu.jpg
The relevance dropdown is too wide

3. https://snipboard.io/apKe35.jpg
Why did you change the order of the filters at the bottom of the adv search page?


4. https://snipboard.io/vq2I57.jpg
The [+][i] links don't behave correctly. This is the screen after 1 click on [+]

5. https://snipboard.io/H3xOZK.jpg
I was surprised by the brightness of the green and noticed that there is a change.
Compare the "Go" button (in the header):
background-color: #85ca11;
background-image: linear-gradient(180deg,#85ca11,#77b50f);
background-position: 0;

With now:
background-color: #71ac0e;
background-image: linear-gradient(180deg,#7dbf0f,#71ac0e);
border-color: #61940c;
box-shadow: 0 0 0 1px #7dbf0f;

Is that expected?
Comment 23 Owen Leonard 2020-09-03 15:15:38 UTC
(In reply to Jonathan Druart from comment #21)
> Also can you confirm the following 3 errors are not valid?
> 
>  FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
>    FAIL   forbidden patterns                                                
>  FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt            
>    FAIL   forbidden patterns                                                

These templates contain links to external (non-Koha) JavaScript assets: LibraryThing and Babeltheque. 

>  FAIL   koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
>    FAIL   forbidden patterns
>                 forbidden pattern: Font Awesome icons need the aria-hidden
> attribute for accessibility purpose (see bug 25166)  (line 124)

That's a valid warning and has been corrected.

(In reply to Jonathan Druart from comment #22)
> 1. https://snipboard.io/9kNAL8.jpg
> Don't you think the login inputs are too wide?

I've added a max-width to the form.
 
> 2. https://snipboard.io/TlIkcu.jpg
> The relevance dropdown is too wide

Fixed.

> 3. https://snipboard.io/apKe35.jpg
> Why did you change the order of the filters at the bottom of the adv search
> page?

I made a lot of changes when working on the responsive behavior of this page. I think the change in the order is unintentional.
 
> 4. https://snipboard.io/vq2I57.jpg
> The [+][i] links don't behave correctly.

Fixed.

> 5. https://snipboard.io/H3xOZK.jpg
> I was surprised by the brightness of the green and noticed that there is a
> change.
> Compare the "Go" button (in the header):

I made changes across the board to buttons to make them more consistent with each other. The buttons in your example screenshot have been made to match the style of the "Go" button in the header. Other buttons, like the "Search" buttons on the advanced search page, have been styled the same where previously they used out-of-the-box Bootstrap "btn-success" colors.
Comment 24 Jonathan Druart 2020-09-04 09:39:42 UTC
The RTL version (tested with ar-Arab) is broken indeed.

First there is a leftover in doc-head-close.inc
 10     [% Asset.css("lib/bootstrap/css/bootstrap-rtl.min.css") | $raw %]
This file has been removed.

Then you can see:
mainpage before: https://snipboard.io/0tgXu7.jpg
mainpage after : https://snipboard.io/uBinZp.jpg

search result before: https://snipboard.io/MjyfO2.jpg
search result after : https://snipboard.io/Mmh0os.jpg

Things to try:
https://bootstrapper.ir/
https://github.com/MahdiMajidzadeh/bootstrap-v4-rtl
https://bootstrap.rtlcss.com/

I don't think we should push this change without RTL support.
Comment 25 Owen Leonard 2020-09-04 15:01:16 UTC
I've pushed an update to my GitLab branch that incorporates this into the gulp process: https://rtlcss.com. This will auto-generate RTL versions of the CSS files generated from SCSS.

I've made some corresponding changes to the way RTL CSS is included on the affected pages.
Comment 26 Jonathan Druart 2020-09-07 08:21:48 UTC
Owen, there is a problem with the dropdown-menu: https://snipboard.io/wcETBt.jpg
Comment 27 Owen Leonard 2020-09-07 18:09:12 UTC
(In reply to Jonathan Druart from comment #26)
> Owen, there is a problem with the dropdown-menu:
> https://snipboard.io/wcETBt.jpg

We talked about this on IRC, but I'll repeat it here for posterity: The problem is that the opaclayoutstylesheet system preference is filled by default with "opac.css." I didn't expect this--I thought the default was blank and that checking for the existence of a value in the preference was enough to determine whether a custom stylesheet was being used.

We could change the way the default it set -- I'm not sure there is a reason to put a value in the preference by default.

I could also modify the check in doc-head-close.inc. Where it is currently:

[% IF ( Koha.Preference('opaclayoutstylesheet') ) %] 

We could change it to:

[% IF ( Koha.Preference('opaclayoutstylesheet') !== "opac.css" ) %]
Comment 28 Owen Leonard 2020-09-07 19:06:57 UTC
(In reply to Owen Leonard from comment #27)
> [% IF ( Koha.Preference('opaclayoutstylesheet') !== "opac.css" ) %]

More accurately:

[% IF ( Koha.Preference('opaclayoutstylesheet') && Koha.Preference('opaclayoutstylesheet') != "opac.css" ) %]

The only reason I bring up the idea of changing the default value of opaclayoutstylesheet is because I don't understand why we set it.
Comment 29 Jonathan Druart 2020-09-08 12:48:54 UTC
I don't see any good reasons to have is set to "opac.css", however I would add the test in the template instead of modifying the pref (can be a follow-up bug report).
Comment 30 Jonathan Druart 2020-09-09 12:37:56 UTC
I've decided to push this (with 2 follow-ups) to have time for catching issues before the 20.11 release.
Comment 31 Jonathan Druart 2020-09-09 12:38:14 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 32 Lucas Gass 2020-09-14 21:56:01 UTC
enhancement will not be backported to 20.05.x series