| Summary: | Update of the OPAC bootstrap template to bootstrap v4 | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Elias <elias.dehler> |
| Component: | OPAC | Assignee: | Owen Leonard <oleonard> |
| Status: | CLOSED FIXED | QA Contact: | Jonathan Druart <jonathan.druart> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | arslanone, dcook, jonathan.druart, josef.moravec, lucas, marjorie.barry-vila, martin.renvoize, mtj, tomascohen, victor |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | https://gitlab.com/koha-dev/koha-dev/-/tree/wip-2020-05-14-opac-bootstrap-4 | ||
| Change sponsored?: | --- | Patch complexity: | Large patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
This patch upgrades the version of the Bootstrap library used by the OPAC from version 2.3.1 to version 4.5.0. The Bootstrap library provides a framework of JavaScript and CSS to support responsive layouts and interface elements like toolbars, menus, buttons, etc. Although some aspects of the interface have been updated, the changes are largely invisible to the user.
|
Version(s) released in: |
20.11.00
|
| Circulation function: | |||
| Bug Depends on: | 14862 | ||
| Bug Blocks: | 25771, 25775, 26164, 26419, 26465, 26478, 26505, 26512, 26519, 26665, 26720, 26735, 26749, 26752, 26758, 26810, 27072, 27200, 27814, 27881, 28193, 28537, 28679, 28868, 29329 | ||
|
Description
Elias
2018-02-09 12:34:53 UTC
Comment hidden (obsolete)
Please base your work on master! The patch would not be accepted on a stable release branch. Consider also how your work might conflict with these in-progress patches: https://bugs.koha-community.org/bugzilla3/buglist.cgi?bug_status=Needs%20Signoff&bug_status=Signed%20Off&bug_status=Passed%20QA&component=OPAC&list_id=228043&query_format=advanced Please also avoid one big change patch - smaller steps are always easier to test and integrate! Not sure, maybe it would make sense to split jQuery update and Bootstrap update into 2 separate bugs and patch sets? *** Bug 18031 has been marked as a duplicate of this bug. *** (In reply to Elias from comment #0) > I'm currently working on updating the bootstrap template to bootstrap v4. Is this something you're still working on? 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' Hi Owen, is this ready for testing or are you still working on it? 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. 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. 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) 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}&title={TITLE}&st=xl&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>
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? (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. 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 :) 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";
^
yarn install yarn build yarn build --view=opac all works ok for me on KTD 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. 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 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 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. 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) 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? (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. 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.
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. Owen, there is a problem with the dropdown-menu: https://snipboard.io/wcETBt.jpg (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" ) %] (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. 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). I've decided to push this (with 2 follow-ups) to have time for catching issues before the 20.11 release. Pushed to master for 20.11, thanks to everybody involved! enhancement will not be backported to 20.05.x series |
