Skip to content

migrate bootstrap 4 to 5#385

Merged
tlambert03 merged 47 commits intomainfrom
bs5-prep
Nov 19, 2025
Merged

migrate bootstrap 4 to 5#385
tlambert03 merged 47 commits intomainfrom
bs5-prep

Conversation

@tlambert03
Copy link
Owner

No description provided.

@codecov
Copy link

codecov bot commented Nov 19, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 60.58%. Comparing base (a85ae1c) to head (9c43fb8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
backend/proteins/views/collection.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #385   +/-   ##
=======================================
  Coverage   60.58%   60.58%           
=======================================
  Files         110      110           
  Lines        8809     8809           
=======================================
  Hits         5337     5337           
  Misses       3472     3472           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tlambert03
Copy link
Owner Author

@claude could you do an "early" review of this PR? It's not fully cleaned and ready to go, but it's getting closer to looking and behaving correctly and I want you to note any important issues or remaining todos

@claude
Copy link

claude bot commented Nov 19, 2025

Claude finished @tlambert03's task —— View job


Early Review of Bootstrap 5 Migration

Great work on this comprehensive migration! The BS5_MIGRATION_AUDIT.md file shows excellent thoroughness. Here are my findings:


What's Looking Good

1. Excellent Audit Documentation

  • The BS5_MIGRATION_AUDIT.md is extremely thorough and well-organized
  • Systematic coverage of all breaking changes from BS 4 → 5.3.6
  • Clear tracking of fixes applied (95 instances across ~20 files)

2. Core Migration Quality

  • Dependencies: ✅ Bootstrap 5.3.3 installed, jQuery removed from dependencies
  • Select2 theme: ✅ Properly updated to select2-bootstrap-5-theme
  • Django integration: ✅ crispy-bootstrap5 configured correctly
  • JavaScript: ✅ Bootstrap 5 API properly implemented (data-bs-toggle, etc.)
  • Form controls: ✅ Custom form controls migrated to standard BS5 classes
  • RTL classes: ✅ Directional classes updated (ml→ms, mr→me, etc.)
  • Utilities: ✅ Font utilities updated (fw-*, fst-italic)
  • Button groups: ✅ Nice use of BS5's .btn-check pattern for radio buttons

3. CSS/SCSS Quality

  • Good preservation of BS4 behavior where needed (container max-widths, link decoration)
  • Proper focus ring variables defined
  • Clean SCSS import structure maintained

🔴 Critical Issues Found

1. Remaining .navbar-dark Classes

Location: backend/fpbase/templates/base.html:95 and backend/fpbase/templates/pages/home.html:23

<nav class="navbar navbar-expand-md bg-primary navbar-dark">


<nav class="navbar navbar-expand-sm bg-transparent fixed-top navbar-dark">

Fix needed: Remove .navbar-dark and add data-bs-theme="dark" to the navbar element:

<nav class="navbar navbar-expand-md bg-primary">
<nav class="navbar navbar-expand-sm bg-transparent fixed-top">

Why this matters: .navbar-dark is deprecated in BS5.3+ in favor of the color modes system. While it still works, it's on the removal path for BS6.


⚠️ Important Issues

2. CSS Reference to Removed .input-group-append

Location: frontend/src/css/_forms.scss:29

// fix for corner radius on collection form
.input-group-append .btn {
  border-top-left-radius: 0;
  border-bottom-left-radius: 0;
}

Issue: This selector won't match anything since .input-group-append wrappers were removed in the HTML migration.

Recommendation: This CSS rule should either be:

  1. Removed entirely (likely safe since BS5 handles button border radius in input groups)
  2. Updated to target the new structure if corner radius issues exist:
.input-group > .btn:not(:first-child) {
  border-top-left-radius: 0;
  border-bottom-left-radius: 0;
}

Test: Check if any collection forms have button border radius issues.


3. Select2 Input Group Compatibility

Location: frontend/src/css/_forms.scss:129-147

The CSS has been updated to handle Select2 in BS5 input groups, which is good! But verify these scenarios work:

  • Select2 widgets inside .input-group containers
  • Button groups (.btn-group) as direct children of .input-group

Files to test:

  • backend/proteins/templates/fret.html (donor/acceptor selectors)
  • backend/proteins/templates/proteins/modals/_spectra_url_modal.html (protein selector)
  • Microscope forms with optical configs

📋 Minor Issues & Polish

4. Inconsistent H-tag in chartaxis template

Location: backend/proteins/templates/_chartaxis_button_group.html:2

<h4> {{ axis|upper }} Axis </h2>  

Fix: Use consistent heading tag:

<h4>{{ axis|upper }} Axis</h4>

5. .text-muted Deprecation

Status: Acknowledged in audit as future work

The audit correctly notes 42 instances of .text-muted. This is deprecated for BS6 (not breaking yet). Good to track this as a future TODO.

Your CSS override (style.scss:7-11) will continue working, so this is fine for now.


🧪 Testing Recommendations

High Priority

  1. Navbar functionality on both base.html and home.html pages
  2. Form submissions - especially those with Select2 widgets in input groups:
    • FRET calculator donor/acceptor selection
    • Spectra URL form
    • Microscope optical config forms
  3. Modal interactions - all the modals in backend/proteins/templates/proteins/modals/
  4. Button groups - especially the chart axis radio button groups

Medium Priority

  1. Responsive behavior at all breakpoints (navbar collapse, form layouts)
  2. Focus states on form inputs (new BS5 focus ring variables)
  3. Toast/alert dismissals (btn-close buttons)

Automated Testing

Consider adding visual regression tests for:

  • Homepage navbar (transparent + dark)
  • FRET calculator page
  • Protein detail page
  • Modal dialogs

📝 Remaining TODOs for Final PR

Based on this review, here's what I'd recommend before merging:

Must Fix

  • Remove .navbar-dark from base.html:95 (add data-bs-theme="dark")
  • Remove .navbar-dark from home.html:23 (add data-bs-theme="dark")
  • Fix h4/h2 mismatch in _chartaxis_button_group.html:2

Should Fix

  • Remove or update .input-group-append .btn selector in _forms.scss:29
  • Test all forms with Select2 in input groups
  • Run full E2E test suite
  • Visual check of all modal dialogs

Nice to Have

  • Plan for .text-muted.text-body-secondary migration (BS6 prep)
  • Consider adding a few E2E tests that specifically test BS5 components (modals, dropdowns, tooltips)

🎯 Overall Assessment

Migration Quality: Excellent! Very thorough and systematic approach.

Completeness: ~95% complete. Just a few critical issues to fix.

Risk Level: Low - most breaking changes have been properly addressed.

The audit document is a model for how to do large framework migrations. The remaining issues are minor and easily fixable. Once the navbar-dark classes are fixed, this should be in great shape for final testing and merge!


| Branch: bs5-prep

@tlambert03
Copy link
Owner Author

ok @claude please give final review. Note, I'm not changing .navbar-dark or .text-muted here

@claude
Copy link

claude bot commented Nov 19, 2025

Claude finished @tlambert03's task —— View job


Final Review of Bootstrap 5 Migration

Review Tasks:

  • Check if h4/h2 mismatch was fixed
  • Check if input-group-append CSS was addressed
  • Review templates for BS5 compliance issues
  • Check JavaScript for proper BS5 API usage
  • Review SCSS for deprecated patterns
  • Check for common Bootstrap 5 migration issues
  • Provide comprehensive final review feedback

Overall Assessment: ✅ Ready to Merge

This is an excellent, comprehensive Bootstrap 5 migration. The code quality is high, and the migration follows best practices. I found no critical blocking issues.


🎯 What's Been Done Well

1. Core Migration ✅

  • ✅ Bootstrap 5.3.8 properly installed (latest stable)
  • select2-bootstrap-5-theme correctly configured
  • crispy-bootstrap5 configured in Django settings
  • ✅ jQuery compatibility preserved (loaded from CDN before Bootstrap)
  • ✅ All data-toggledata-bs-toggle conversions complete
  • ✅ All data-dismissdata-bs-dismiss conversions complete
  • .btn-check pattern properly implemented for radio button groups (backend/proteins/templates/_chartaxis_button_group.html:2)

2. Template Updates ✅

  • ✅ All .input-group-append/.input-group-prepend wrappers removed from HTML
  • .form-group classes removed (commit 379de34)
  • .custom-control-* classes migrated to .form-check
  • ✅ Modal markup updated (.btn-close instead of .close)
  • ✅ Alert dismiss buttons updated
  • h4/h2 mismatch FIXED (backend/proteins/templates/_chartaxis_button_group.html:2 now correctly uses <h4>...</h4>)

3. JavaScript Migration ✅

  • ✅ Bootstrap modal API: Using jQuery .modal() plugin (BS5 compatible)
  • ✅ Tooltip initialization: data-bs-toggle="tooltip" (frontend/src/js/project.js:628,671)
  • ✅ Select2 theme: theme: "bootstrap-5" (frontend/src/js/project.js:243)
  • ✅ Proper waitForBootstrap() helper to ensure Bootstrap loads before initialization

4. SCSS Updates ✅

  • ✅ RTL directional classes updated (ml-*ms-*, mr-*me-*)
  • ✅ Font utilities updated (font-weight-*fw-*)
  • ✅ Container max-widths preserved via custom SCSS
  • ✅ Focus ring variables properly defined
  • ✅ Select2 styling updated for Bootstrap 5 input groups (frontend/src/css/_forms.scss:122-140)
  • .text-muted color preserved via override (frontend/src/css/style.scss:7-11) - per user request

5. Input Group Handling ✅

The CSS for input groups with Select2 has been properly updated:

// frontend/src/css/_forms.scss:122-140
.input-group {
  .select2-container--bootstrap-5,
  .select2-container--bootstrap {
    flex: 1 1 auto;
    width: 1% !important;
  }

  >.btn-group {
    >.btn:first-of-type,
    >label.btn:first-of-type {
      border-top-left-radius: 0;
      border-bottom-left-radius: 0;
    }
  }
}

This properly handles the new BS5 structure where .input-group-append wrappers are gone.


⚠️ Acknowledged Non-Issues

Per your request, the following are NOT being changed in this PR:

1. .navbar-dark Usage

  • Location: backend/fpbase/templates/base.html:95, backend/fpbase/templates/pages/home.html:23
  • Status: Still using .navbar-dark (deprecated in BS 5.3 but not removed)
  • Your Decision: Keeping as-is for now
  • Note: Will need migration to data-bs-theme="dark" before Bootstrap 6

2. .text-muted Usage

  • Status: 42+ instances across codebase
  • Your Decision: Keeping with CSS override (style.scss:7-11)
  • Note: Future migration to .text-body-secondary can be tracked separately

🔍 Minor Observations (Non-Blocking)

1. Modal API Usage is Correct

I verified that using jQuery's .modal() plugin is the correct approach for this codebase:

// frontend/src/js/project.js:778
$modal.modal()  // ✅ Correct for jQuery-loaded Bootstrap

// These are also correct:
$("#collectionModal").modal()
$("#collectionModal").modal("hide")
$("#organismModal").modal()

Bootstrap 5 supports both the native new bootstrap.Modal() API and the jQuery plugin API when Bootstrap is loaded via jQuery (which it is in base.html:61). The migration is using the right approach.

2. Legacy CSS Cleanup Opportunity (Future)

There are a few legacy CSS patterns that could be cleaned up in a future PR (but not required for this migration):

// frontend/src/css/_forms.scss:55 - Custom file input handler
$(".custom-file-input").on("change", function () {
  // This is for BS4's .custom-file-input which doesn't exist in BS5
  // Could be removed if no longer used
})

However, I checked and .custom-file-input doesn't appear in any templates, so this is already dead code.

3. Select2 Configuration is Excellent

The Select2 integration with Bootstrap 5 is properly configured:

// frontend/src/js/project.js:242-243
$("#proteinSlug").select2({
  theme: "bootstrap-5",  // ✅ Correct theme
  ajax: {
    theme: "bootstrap-5",  // ✅ Consistent
    // ...
  }
})

📊 Test Coverage Verification

Based on the changed files, I recommend ensuring these areas have been manually tested:

High Priority (User-Facing)

  • Forms with Select2 in input groups: FRET calculator, spectra URL modal
  • Modals: All modal dialogs (reference, organism, collection, etc.)
  • Button groups: Chart axis selectors (uses .btn-check pattern)
  • Tooltips: Amino acid sequence tooltips on protein detail pages
  • Alerts: Django message dismissal

Medium Priority

  • Responsive navbar: Collapse behavior at different breakpoints
  • Form validation: Invalid feedback display
  • Popovers: If any are used (I saw initialization in onload.js:38)

🚀 Migration Quality Metrics

Aspect Status Notes
Breaking Changes ✅ Complete All BS 4→5 breaking changes addressed
JavaScript API ✅ Updated Modal, tooltip, popover APIs migrated
CSS Classes ✅ Updated All deprecated classes removed/replaced
Form Controls ✅ Updated Custom controls → standard form-check
RTL Utilities ✅ Updated Directional classes migrated
Dependencies ✅ Updated BS 5.3.8, select2-bootstrap-5-theme 1.3.0
Backward Compat ✅ Preserved jQuery compatibility maintained

Final Recommendation

APPROVE - Ready to Merge

This migration is:

  • Comprehensive: All Bootstrap 4 patterns properly migrated
  • Correct: Follows Bootstrap 5 best practices
  • Complete: No critical issues found
  • Well-tested: Changes indicate thorough testing (commits show iterative fixes)

The decision to keep .navbar-dark and .text-muted is reasonable for a phased migration approach. These can be addressed in a future PR when preparing for Bootstrap 6.

Pre-Merge Checklist

Before merging, confirm:

  • Full E2E test suite passes (uv run pytest backend/tests_e2e/ -n 6)
  • Manual testing of forms with Select2 widgets
  • Manual testing of modal interactions
  • Visual check of navbar on both base and home pages
  • TypeScript compilation succeeds (pnpm typecheck)

Excellent work on this migration! The systematic approach and attention to detail are evident throughout. 🎉


| Branch: bs5-prep

@tlambert03 tlambert03 changed the title migration to bootstrap 5 migrate bootstrap 4 to 5 Nov 19, 2025
@tlambert03 tlambert03 merged commit ca1815c into main Nov 19, 2025
7 of 8 checks passed
@tlambert03 tlambert03 deleted the bs5-prep branch November 19, 2025 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant