rubyforgood / human-essentials

Human Essentials is an inventory management system for diaper, incontinence, and period-supply banks. It supports them in distributing to partners, tracking inventory, and reporting stats and analytics.
https://humanessentials.app
MIT License
465 stars 494 forks source link

[Investigation] Current bootstrap (4) is past EOL…upgrade to Bootstrap 5 #3450

Closed cielf closed 1 year ago

cielf commented 1 year ago

Summary

To stay current with the most recent bootstrap APIs and functionality we need to upgrade from bootstrap 4 to bootstrap 5. The challenge is that we are currently using a theme, AdminLTE, but that theme has not released a bootstrap 5 version yet. We like someone to do a research spike and see if the current working bootstrap 5 branch of AdminLTE is ready enough for us to use and see if we are able to move to the newest version of bootstrap.

Why update?

Very minor potential for security issues due to no longer getting patches.

Primarily though, having such an outdated version of a primary library will make it more difficult for new contributors to dive in and help out as bootstrap continues to evolve and add new and different classes/ids/etc that contributors will expect to exist but are not present on the prior version of bootstrap.

Details

Here is a link to the bootstrap 5 branch of AdminLTE. Here is a link to the bootstrap 5 upgrade documentation.

Notes:

Just a reminder that the application is now using importmaps.

Initial step:

Determine the status of Admin LTE support for Bootstrap 5. E.g., how far away is the in-progress branch from supporting what we need? It is possible that since AdminLTE supports multiple frameworks (React, Vue, etc) that the base stylings are already finished.

Criteria for completion

ChaelCodes commented 1 year ago

Investigation

Here's some initial results from the investigation into upgrading Bootstrap from the DC event on 7/28.

ADR

There's an architecture decision record from Oct 2022 where Tailwind was considered, but declined due to the effort involved in migrating. https://github.com/rubyforgood/human-essentials/blob/main/doc/architecture/decisions/0009-stick-with-adminlte-for-app-design.md

Importing

We import AdminLTE using importmaps. https://github.com/rubyforgood/human-essentials/blob/9df05d4b0f9452b4657d9daeeabb7c03f8ccc0b2/config/importmap.rb#L4 This line imports a local file - https://github.com/rubyforgood/human-essentials/blob/main/app/javascript/adminlte.js containing 3000+ lines. It's unclear how bootstrap is loaded here.

There's css imported as well in https://github.com/rubyforgood/human-essentials/blob/main/app/assets/stylesheets/AdminLTE.css

Bootstrap is imported in the gemfile, but the connection to AdminLTE is unclear. https://github.com/rubyforgood/human-essentials/blob/9df05d4b0f9452b4657d9daeeabb7c03f8ccc0b2/Gemfile#L43

Usage

There are at least 4 files labeled lte:

jamesiarmes commented 1 year ago

Picking this up along with @h-m-m.

awwaiid commented 1 year ago

Per the PR -- bootstrap is upgraded! But we need to do a visual regression test (and likely fix a conflict or two as we are doing parallel work). The PR has a big checklist of areas / types of things to check.

github-actions[bot] commented 1 year ago

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

github-actions[bot] commented 1 year ago

Automatically unassigned after 7 days of inactivity.