[VUFIND-1746] Drop bs3-compat.js #4224

EreMaijala · 2025-01-31T09:53:33Z

This set of changes eliminates the need for bs3-compat.js in VuFind's templates, other JS and styles.

TODO:

Mark VUFIND-1746 resolved when merged.
Add a change log entry describing the changes and note that adjustments may be needed in local templates if not already done in 10.x migration.

demiankatz

Thanks, @EreMaijala, this looks good to me and all tests are passing -- it just raised a few minor questions and ideas.

I'm also going to ask @sturkel89 to take a look at this, since her attentive eye might spot something visual that the test suite is overlooking.

For @sturkel89: here is a list of some of the areas where things have changed, so you can focus your testing accordingly. I think generally speaking we want to be sure that things look normal/okay in both bootstrap5 and sandal5. I realize that this could take a really long time to work through, so I don't necessarily expect a thorough attack on every angle. I'm also happy to elaborate on anything I mention that's unclear or that you haven't tested before. Might be helpful for @EreMaijala to highlight whether there are things here that he didn't have time to test himself or has particular concerns about.

Channels (particularly the menu for adding more of them)
Expanding/collapsing things (particularly facets, the summary list when performing cart actions, and the DPLATerms/RandomRecommend modules)
Tab behavior on records and collections
On-screen virtual keyboards
Multiple CAPTCHA behavior
Drop-down menus (e.g. language/theme selectors)
Drop-down options in forms (e.g. in the hierarchy search control, on the admin screens, in course reserves, etc.)
Breadcrumbs on most pages
The mobile hamburger menu in the header
The account side menu
The accordion and tabs views in search results
The home page in the local_theme_example theme

themes/bootstrap5/js/common.js

themes/bootstrap5/scss/bs3-compat.scss

demiankatz · 2025-01-31T13:53:24Z

themes/bootstrap5/templates/collection/view.phtml

@@ -26,11 +26,11 @@
  // Set up breadcrumbs:
  $lastSearch = $this->searchMemory()->getLastSearchLink($this->transEsc('Search'));
  if (!empty($lastSearch)) {
-      $this->layout()->breadcrumbs = '<li>' . $lastSearch . '</li> ';
+      $this->layout()->breadcrumbs = '<li class="breadcrumb-item">' . $lastSearch . '</li> ';


There has been talk in the past about creating a breadcrumbs view helper. All this formal markup across the templates makes me think the need for that may now be even greater than before. What do you think? Is this worth putting on the to-do list somewhere? Maybe we should look at the ancient discussion on VUFIND-747 and see if it's worth charting a new course forward.

Yes, it would make a lot of sense, and it's not nice that we have the markup scattered all over the templates.

I've just left some thoughts on that ticket -- we can move further discussion over there.

demiankatz · 2025-01-31T13:58:47Z

themes/bootstrap5/templates/myresearch/account.phtml

 ?>
 <h2><?=$this->transEsc('User Account')?></h2>
 <?=$this->flashmessages()?>

-<form method="post" name="accountForm" id="accountForm" class="form-user-create" data-bs-toggle="validator">
+<form method="post" name="accountForm" id="accountForm" class="form-user-create" data-toggle="validator">


Just trying to understand what's going on here: was the old data-bs-toggle a typo? I thought BS5 added more -bs- specifiers to attributes, so I'm curious why this is being removed. Is this not a Bootstrap mechanism at all?

Yes, it was a mistake. Validator is not part of Bootstrap but a separate module.

Is that a fix we should consider backporting to release-10.1, then, or does the bs3-compat straighten it out there?

I'll provide a separate fix for release-10.1.

Actually, I'll provide a separate fix for both Bootstrap 3 and Bootstrap 5 for release-10.1, and the Bootstrap 5 part can then be forward-ported to dev. It's better to use Bootstrap's own separate mechanism in Bootstrap 5, and the old validation library is no longer supported anyway.

demiankatz · 2025-01-31T18:28:29Z

Thanks, @EreMaijala, this is looking good to me but I'll wait to approve until @sturkel89 finishes testing (probably early next week). In the meantime, do you want to take a stab at drafting the changelog notes, or should I? Would detailed notes actually belong on the existing bootstrap5 page?

sturkel89

I've tested this PR in both themes and compared with dev. Most functions are fine; I uncovered only two issues that work differently between test and dev and could use some attention.

Tag Management buttons

On Admin screens, under Tag Maintenance, the List Tags and Manage Tags buttons appear as words, rather than buttons, on test in both themes. I'll provide screenshots in Sandal5.

http://localhost/vufind_test/Admin/Tags/Manage

Test:

Dev:

Multiple CAPTCHA options

I set up three CAPTCHA options in config.ini. In the test branch, only the first named CAPTCHA option is enabled; the other two are not clickable. Also, the CAPTCHA options are not displayed as tabs in either lightbox or on full screen.

Test:

On dev, all three CAPTCHA options are enabled. However, in lightbox, the labels are run together rather than being displayed in tabs.

In other words, nowhere in bootstrap5 do lightboxes handle multiple CAPTCHA options as tabs. But in the test branch, an additional problem is that only the first CAPTCHA option can be selected when there are multiple options configured.

EreMaijala · 2025-02-04T11:47:47Z

@sturkel89 Thanks for the feedback. Both issues should now be fixed!

sturkel89

I revisited the places where things weren't quite right yesterday - and they are all fixed! Thanks!

sturkel89 · 2025-02-04T18:12:18Z

FYI - I discovered an obscure little bug while testing this branch: "Show other versions" does several weird things when you're in the accordion view of search results.

The behavior was the same on test and dev, so I didn't report it in my review. I have not looked at an old release to see if the behavior is different in the bootstrap3 environment.

Demian said to open a JIRA ticket so we wouldn't forget about this. Here it is: VUFIND-1753.

demiankatz

Thanks, @EreMaijala and @sturkel89. I think this is ready to merge, but I wasn't sure about the best approach to the changelog notes. @EreMaijala, do you have anything in mind? If not, maybe we can discuss tomorrow before hitting the merge button.

EreMaijala · 2025-02-05T07:08:02Z

@demiankatz I updated the Bootstrap 5 page section about the compatibility layer yesterday. I think that the change log entry could point to that. Perhaps something like this right after the entry about removal of Bootstrap 3 themes:

Also the Bootstrap 3 compatibility layer has been removed. If your local theme depends on Bootstrap 3 -specific markup, please upgrade to 10.1.x first and apply the required changes to eliminate the need for the compatibility layer first before upgrading to release 11.0. See the relevant section on the Bootstrap 5 page for more information.

demiankatz · 2025-02-05T13:17:24Z

Thanks, @EreMaijala, I have updated the changelog as you suggested!

(cherry picked from commit 854992f)

(cherry picked from commit 854992f) (cherry picked from commit f3c3644)

EreMaijala added 13 commits January 30, 2025 09:32

Drop bs3-compat from navbar.

9c2b54d

Drop bs3-compat from navs and dropdowns.

b8a2787

Drop Bootstrap version checks from JS.

b07d879

Fix header account menu.

7d9e416

Add form-select class to all select fields and drop JS from bs3-compat.

5e7e766

Drop bs3-compat JS requirement for breadcrumbs.

a72f4d2

Drop bs3-compat JS requirement for collapse.

920bc06

Drop bs3-compat JS requirement for pagination.

9fb8c8f

Fix data-toggle="validator"

6c39fbc

Drop remaining bs3-compat JS.

a86fa4f

Fix collection and record page breadcrumbs.

7f685ed

Merge remote-tracking branch 'origin/dev' into dev-cleanup-bs3-compat

c8b6206

Drop useless rule for facet collapse buttons.

ae8efd0

EreMaijala requested a review from demiankatz January 31, 2025 09:53

demiankatz requested changes Jan 31, 2025

View reviewed changes

demiankatz requested a review from sturkel89 January 31, 2025 14:09

demiankatz added this to the 11.0 milestone Jan 31, 2025

demiankatz added the architecture pull requests that involve significant refactoring / architectural changes label Jan 31, 2025

EreMaijala added 2 commits January 31, 2025 16:16

Remove getBootstrapMajorVersion function.

1ee355d

Revert validator toggle change.

86b6e4a

EreMaijala requested a review from demiankatz January 31, 2025 16:56

sturkel89 requested changes Feb 3, 2025

View reviewed changes

Fix tags admin menu and captcha tabs.

ce48c01

EreMaijala requested a review from sturkel89 February 4, 2025 11:47

Merge branch 'dev' into dev-cleanup-bs3-compat

4887072

sturkel89 approved these changes Feb 4, 2025

View reviewed changes

demiankatz approved these changes Feb 4, 2025

View reviewed changes

demiankatz merged commit 854992f into vufind-org:dev Feb 5, 2025
6 checks passed

demiankatz deleted the dev-cleanup-bs3-compat branch February 5, 2025 13:17

demiankatz mentioned this pull request Feb 5, 2025

Accessibility: Removes hard-coded columns in home facet lists #4231

Open

EreMaijala added a commit to NatLibFi/NDL-VuFind2 that referenced this pull request Feb 7, 2025

[VUFIND-1746] Drop bs3-compat.js (vufind-org#4224)

f3c3644

(cherry picked from commit 854992f)

EreMaijala added a commit to NatLibFi/NDL-VuFind2 that referenced this pull request Feb 10, 2025

[VUFIND-1746] Drop bs3-compat.js (vufind-org#4224)

05dd8de

(cherry picked from commit 854992f) (cherry picked from commit f3c3644)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[VUFIND-1746] Drop bs3-compat.js #4224

[VUFIND-1746] Drop bs3-compat.js #4224

EreMaijala commented Jan 31, 2025 •

edited by demiankatz

Loading

demiankatz left a comment

demiankatz Jan 31, 2025

EreMaijala Jan 31, 2025

demiankatz Jan 31, 2025

demiankatz Jan 31, 2025

EreMaijala Jan 31, 2025

demiankatz Jan 31, 2025

EreMaijala Jan 31, 2025

EreMaijala Jan 31, 2025

EreMaijala Feb 3, 2025

demiankatz commented Jan 31, 2025

sturkel89 left a comment

EreMaijala commented Feb 4, 2025

sturkel89 left a comment

sturkel89 commented Feb 4, 2025

demiankatz left a comment

EreMaijala commented Feb 5, 2025

demiankatz commented Feb 5, 2025

[VUFIND-1746] Drop bs3-compat.js #4224

[VUFIND-1746] Drop bs3-compat.js #4224

Conversation

EreMaijala commented Jan 31, 2025 • edited by demiankatz Loading

demiankatz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

demiankatz commented Jan 31, 2025

sturkel89 left a comment

Choose a reason for hiding this comment

Tag Management buttons

Multiple CAPTCHA options

EreMaijala commented Feb 4, 2025

sturkel89 left a comment

Choose a reason for hiding this comment

sturkel89 commented Feb 4, 2025

demiankatz left a comment

Choose a reason for hiding this comment

EreMaijala commented Feb 5, 2025

demiankatz commented Feb 5, 2025

EreMaijala commented Jan 31, 2025 •

edited by demiankatz

Loading