Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
3 tasks
cielf opened this issue Mar 14, 2023 · 5 comments · Fixed by #3779
Closed
3 tasks

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

cielf opened this issue Mar 14, 2023 · 5 comments · Fixed by #3779
Labels
Ruby for Good 2023 DC Created for Ruby for Good 2023 DC stale

Comments

@cielf
Copy link
Collaborator

cielf commented Mar 14, 2023

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

  • Investigate current state of Admin LTE vis a vis Bootstrap 5 -- how far away is it from supporting what we need?
  • If possible, make a branch with one of the layout pages converted and pulling the new assets -- application.html.erb, _lte_sidebar.html.erb, _lte_navbar.html.erb as well as a single page in the application. Link to the branch as well as screenshots.
  • Document any potential issues with the issue/upgrade/
@cielf cielf added Help Wanted Groomed + open to all! core team Groomed but likely needs expert knowledge labels Mar 14, 2023
@seanmarcia seanmarcia removed the core team Groomed but likely needs expert knowledge label Mar 14, 2023
@cielf cielf added Ruby for Good 2023 DC Created for Ruby for Good 2023 DC and removed Help Wanted Groomed + open to all! labels Jun 7, 2023
@ChaelCodes
Copy link
Contributor

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.

pin "admin-lte", to: "adminlte.js", preload: true

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.

gem 'bootstrap', '~> 4.6.0'

Usage

There are at least 4 files labeled lte:

@jamesiarmes
Copy link
Member

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

@awwaiid
Copy link
Collaborator

awwaiid commented Jul 30, 2023

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
Copy link
Contributor

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 github-actions bot added the stale label Aug 30, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Automatically unassigned after 7 days of inactivity.

awwaiid added a commit that referenced this issue Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ruby for Good 2023 DC Created for Ruby for Good 2023 DC stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants