Update to Bootstrap 5

[kinow]

Closes #1045

• ported Bootstrap 3 to Bootstrap 5
• updated dependencies fixing the stylesheet and JS accordingly
• added CSS variables
• tested
• addressed feedback & fixed issues (testing again)
• wrote a changelog suggested entry
• tested autocomplete (Autocomplete component does not highlight the search term as a matching part when using decomposed unicode characters #1246)

[kinow]

Note to self: before making it a final PR, confirm whether there's a new version of twbs/bootstrap and update here if necessary.

[kinow]

Note to self: this is to speed up development, mapping the Docker volume. Undo all the changes to the Dockerfiles when the PR is ready for review.

[kinow]

Glyphicons is not free anymore, and Bootstrap dropped it in Bootstrap 4. I've used this SVG from Google Material Icons, but we can use another one instead too 👍

[kinow]

@osma I believe this PR is ready for an initial round of reviews. Here's how I tested:

# one terminal for:
cd skosmos/dockerfiles
docker-compose rm -f && docker-compose up --force-recreate

# another terminal with files for UNESCO and STW vocabs
curl': curl -I -X POST -H Content-Type:text/turtle -T unescothes.ttl -G http://localhost:9030/skosmos/data --data-urlencode graph=http://skos.um.es/unescothes/ && ^Crl -I -X POST -H Content-Type:text/turtle -T stw.ttl -G http://localhost:9030/skosmos/data --data-urlencode graph=http://zbw.eu/stw/

You'll notice I've changed the dockerfiles contents in this PR, but that's only for this review. I'll remove/edit commits later as that's only useful for development. I'm basically binding a volume so I can quickly develop the changes in the container and watch the changes in the browser.

In one monitor then I have http://localhost:9090 with Skosmos + Bootstrap v5, and in another monitor I have skosmos.dev.finto.fi/en/ with Skosmos + Bootstrap v3.

But this is not the only way to review. If you have a working PHP installation and you are more comfortable using it, feel free to check out this branch and try it. The docker changes should not interfere with your review 👍

Thanks!
Bruno

[osma]

Thanks a lot @kinow , I will review this and get back to you soon!

[osma]

I tested this locally (no docker involved). I was surprised about how smooth the upgrade seems to be, even though this is skipping directly from Bootstrap 3 to 5!

That said, I found some issues. I used Firefox 90 on Ubuntu 20.04 for testing.

Functional:

1. (This could be related to the Typeahead upgrade you did in this PR.) When inside a vocabulary, the search box is doing an unrestricted global search instead of searching within the vocabulary and the chosen language. This means the search is slow and returns results also from other vocabularies, e.g.
   
   I checked the AJAX query and it looks like this: rest/v1/search?query=aardva* while in the old version it looks like this: rest/v1/search?query=aardv*&vocab=yso&lang=en&labellang=en - so clearly the URL parameters are not being set correctly. The same happens also on the front page where the search is (by default) global; the query is just rest/v1/search?query=aardva* when it should be e.g. rest/v1/search?query=aardv*&vocab=&lang=en&labellang=en

Display-related:

1. The Fira font is not being applied to many elements. Here are some examples where I'm seeing another font (Arial?) instead:

These were just some obvious ones, there could be more.

2. I tested how the layout works on narrower devices using Responsive Design mode. There seems to be a lot of empty space on e.g. the vocabulary information page:

3. In the search bar, the font size has increased a little bit.

4. The copy to clipboard icon - I understand the issue with Glyphicons. I think we may want to choose a different icon than the one you selected though. I will get back about this. Also, I think it would be better to avoid embedding the SVG icon in the HTML code. The icon is already used in a couple of places and there could be more in e.g. Skosmos plugins, so either a CSS rule (like glyphicons) or a separate image file would be better.

Keep up the good work!

[kinow]

Hi @osma ! Thanks for the excellent feedback (as always!). I was looking only at CSS changes and didn't see the URL query parameters change, nice catch! Will go through your feedback in the next days and update this PR.

[kinow]

(This could be related to the Typeahead upgrade you did in this PR.) When inside a vocabulary, the search box is doing an unrestricted global search instead of searching within the vocabulary and the chosen language. This means the search is slow and returns results also from other vocabularies

Fixed. You were correct @osma , Typeahead changed its syntax, so I had to read their docs and update the code. I beleive now the result should be the same as before 👍

[kinow]

The Fira font is not being applied to many elements. Here are some examples where I'm seeing another font (Arial?) instead:

Bootstrap 5 has a CSS variable that is setting the font to the OS default sans-serif font. There's also a reset.css or reboot.css I think, that resets other font attributes. The issue is that in Bootstrap 3 we didn't have that, so now Bootstrap 5 is trying to be helpful by resetting font attributes, but overriding some of what we had defined.

I've added the font-family: inherit and then it got the font-family from that CSS variable. Also had set the font-family for h2 and h3 only, now p, h_ are all using Fira. But will have to have another look later to confirm I didn't miss any HTML elements.

[kinow]

Three more to go! Will have to stop for today, but will pick it up in the next day(s).

[kinow]

In the search bar, the font size has increased a little bit.

Fixed setting the document root font size. Usually I try to avoid touching the root properties, but Bootstrap is doing the same, but now with a bigger font. We don't have SASS or SCSS in our build toolchain, otherwise I could alter Bootstrap's variable. For now I think this will do (later if the UI build changes it should be simpler to update it).

[kinow]

The copy to clipboard icon - I understand the issue with Glyphicons. I think we may want to choose a different icon than the one you selected though. I will get back about this.

👍

Also, I think it would be better to avoid embedding the SVG icon in the HTML code. The icon is already used in a couple of places and there could be more in e.g. Skosmos plugins, so either a CSS rule (like glyphicons) or a separate image file would be better.

Good idea. Moved it to an external file, so we can just swap the file and it should be updated.

[kinow]

I tested how the layout works on narrower devices using Responsive Design mode. There seems to be a lot of empty space on e.g. the vocabulary information page:

Hmm, you are right. I was using Firefox CTRL+M to test the responsive mode, but with a very small viewport. Looks like the medium viewport breaks the UI. Bootstrap 4 & 5 have a container-fluid exactly for cases like that, so I switched our .content by .content-fluid, and added a few more rules to try to match the Bootstrap 3 rules, keeping as close as possible to the old layout.

I think I addressed your initial feedback @osma. Let me know if there are other items pending, otherwise I will have another look at the differences one day of this week (so I stop comparing it for a while; will choose a day when 🧠 is happy and fresh, so I'm able to spot more differences).

Cheers
Bruno

[osma]

Thanks for the fixes! I did another round of testing, using Firefox 91 on Ubuntu. This time the issues are more minor, in many cases just small adjustments of a few pixels. Mind you, not everything has to be pixel-perfect in the upgrade - I think it's OK to change things in the layout as long as they don't look bad or cause usability or accessibility issues.

1. The hover color on the sidebar tab looks wrong (I couldn't get the mouse cursor included in the screenshot):
   

2. In the sidebar, the tabs are a couple of pixels higher than they used to be. Also, the alphabetical letters are spaced further apart. In this screenshot (old on the left, new on the right) you can see that L is the first letter in the second row, when it used to be the last one in the first row:
   

3. The search bar is a few pixels wider than it used to be, but the search box itself is actually a bit narrower. The down arrow in the content language selection is further away from the language name and higher up. The X in the search box is lower down. In the screenshot, the old is on top, new on the bottom:
   

4. On the vocabulary front page (but not other page types), the left margin on the content area is narrower than it used to be. Also the gap between the columns is narrower and the column widths are different. I think it would be important to maintain the margins and the width of the gap; the exact column widths are not so important as they are affected by the content anyway.
   

5. In the content area of the concept page, there is more vertical space between lines than before, especially just above and below PREFERRED TERM but also between later rows. For example the screenshots below cover the exact same elements, but the new one (on the right) is 30 pixels higher:
   

6. The search suggestion box that pops up is a couple pixels too wide on the left side:
   

7. The feedback page is missing the white background it used to have:
   

8. The download links on the vocabulary front page are not shown using the Fira Sans font - only Fira should be used in Skosmos. This is actually a bug in the current version as well, so not your fault! ;) But maybe we could fix this in this Bootstrap upgrade.
   

Phew, that's what I found for now. Keep up the good work!

[kinow]

Woah! That's an impressive review @osma !! I'll try to update the branch again fixing these bugs. The ones where items are not aligned really bug me, but I missed them sorry. At least I think we are getting close to a working Skosmos+Bootstrap5, and this PR appears to be going in the right direction 🎉 🕺 thanks!!!

[kouralex]

To start, I just want to say that this is an awesome WIP @kinow and I just wanted to give it a try!

Some remarks not yet covered by @osma :

• Autocomplete is currently limited to 5 results only
• There are couple margin/lining/missing information (type) issues in the dropdown, see image below:
  
  *Aligning, font size and background color on front page:
  
  *Alphabetical index does not get initialized on vocabulary front page load

Keep up the good work! :)

[kinow]

Thanks for the review @kouralex ! The more the merrier. Noted the issues found, will start fixing them slowly soon 🤓

[kinow]

@osma, thank you for the feedback.

1. The hover color on the sidebar tab looks wrong (I couldn't get the mouse cursor included in the screenshot):

Good spot! Bootstrap 5 defines the link hover color for navigation links. Had to override that one.

2. In the sidebar, the tabs are a couple of pixels higher than they used to be. Also, the alphabetical letters are spaced further apart. In this screenshot (old on the left, new on the right) you can see that L is the first letter in the second row, when it used to be the last one in the first row:

This was the hardest to tackle. I had noticed this regression, but forgot to point it out, sorry. Basically, Bootstrap 5 uses flex elements to create the pagination. Browsers implement grid & flex nowadays, and it's a lot easier to work with these than trying to align items ourselves (main justifications for moving to grid or flex).

Looks like the flex model adds some kind of virtual border, as the width of the elements was the same in old & new. I reduced the width of the element from 26px to 24px, and now the pagination layout is looking a bit closer to the old design. WDYT @osma?

3. The search bar is a few pixels wider than it used to be, but the search box itself is actually a bit narrower. The down arrow in the content language selection is further away from the language name and higher up. The X in the search box is lower down. In the screenshot, the old is on top, new on the bottom:

Tried to match the previous layout. It's tricky because of the structure changes from Bootstrap 3 to 5. But I think I got the elements re-aligned. Not identical, but the down-arrow should look better aligned. I spent some time trying to understand why I was getting different spacing for the two down-arrows/carets in the screen. Turns out the one in the top had the text "English", and the one next to the search field had "English " with an extra space.

4. On the vocabulary front page (but not other page types), the left margin on the content area is narrower than it used to be. Also the gap between the columns is narrower and the column widths are different. I think it would be important to maintain the margins and the width of the gap; the exact column widths are not so important as they are affected by the content anyway.

Ah, I hadn't noticed it! I caused this bug when using container-fluid which has its own gutter values (padding/margin). I've reverted to .container only, but added width: 100% and padding: 0 15px to match the concept-info.

5. In the content area of the concept page, there is more vertical space between lines than before, especially just above and below PREFERRED TERM but also between later rows. For example the screenshots below cover the exact same elements, but the new one (on the right) is 30 pixels higher:

I think I got it fixed now. The clipboard icon is getting in the way of the row element. I've changed it a bit, but I think we will revisit it later when we decide which icon to use as replacement for glyphicon (now paid).

6. The search suggestion box that pops up is a couple pixels too wide on the left side:

I couldn't find what change caused that, so my best guess is that typeahead changed something in its CSS and caused that? I reduced the width of the suggestion box in 2 pixels, and then moved the box 1 pixel to the right. The border has 1 pixel to each side (top, right, bottom, left), so that should position the suggestion box exactly below with the border and no content overflowing the search input.

7. The feedback page is missing the white background it used to have:

Fixed by one of the changes above. Couldn't reproduce it on Chromium or Firefox. Could you test it again, please @osma?

8. The download links on the vocabulary front page are not shown using the Fira Sans font - only Fira should be used in Skosmos. This is actually a bug in the current version as well, so not your fault! ;) But maybe we could fix this in this Bootstrap upgrade.

Oh, interesting. @osma, I've applied the Fira font to the html and body elements now, as well as the font-size 14px. So it should apply to all elements by default. I think that's the intention? To use that font everywhere? If so, it should fix all font-family issues.

Going to work on @kouralex feedback now. Thanks!

[kinow]

Hi @kouralex,

Autocomplete is currently limited to 5 results only

Turns out there's a limit now in Typeahead, that defaults to 5. I've added a variable in the config.js file set to 100. @osma, @kouralex, should we increase it to something like 1000, or does 100 sound reasonable?

There are couple margin/lining/missing information (type) issues in the dropdown, see image below:

I think @osma had provided a similar feedback. I changed the border/width of the tt-menu, and it looks aligned on chromium/ffox. Could you check if that looks better now? If not, could you share your environment settings, please? Then I will try to reproduce locally.

*Aligning, font size and background color on front page:

I zoomed in and opened two browser windows with old & new skosmos to compare the dropdowns. I tried to match the layout & behaviour. And I must admit it was really different, thanks for pointing the differences @kouralex! Could you take another look and see if it's better, or at least going in the right direction?

*Alphabetical index does not get initialized on vocabulary front page load

I just tested with YSO, STW, and UNESCO vocabularies, and the alphabetical index loaded correctly, I think. Could you confirm if that's still an issue, please, @kouralex?

Thanks a lot for the review!

[osma]

The css_class mechanism, used to add active class to some elements in Twig templates, is a bit broken. The variable is initialized to ['nav-link'] and then an active class is conditionally added. But the active class is never removed, so the class will be applied also to subsequent elements...

I'm trying to think of a quick fix to make it work better.

[osma]

After many little last minute tweaks I think this is now good enough for merging 🎉
I'm sure there will be more adjustments needed, but we can do them separately.
Also a few minor adjustments need to be made to the Finto CSS rules since there are small changes in the HTML DOM structure (mainly the active class that has moved from the li elements to the a child elements), I will implement those as well.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
27 Code Smells

No Coverage information
2.6% Duplication

[osma]

Finto CSS (for dev.finto.fi which is always running current master) adjustments done in this commit

[kinow]

Thank you for reviewing and fixing the PR too @osma ! I'll keep an eye open for Bootstrap & layout issues in Finto & master.