Bootstrap upgrade by ewels · Pull Request #3264 · MultiQC/MultiQC

ewels · 2025-07-05T22:09:47Z

WIP: Updating report front-end

Use Vite (with npm) to build + compile CSS / JS
Upgrade Bootstrap to latest
Try to remove unnecessary JS libraries
Add support for report colour themes

Initial attempt in this PR, but still quite a lot left to do. After an initial burst of speed, I'm expecting this PR to take a while.

To-do

I'll try to add to this list as I think of things. From roughly easy to difficult:

wontfix - push to another day

Include plotly / jQuery etc. into npm build step (instead of keeping in source code)
Investigate removing jQuery UI with native JS?
Look into removing compiled JS / CSS from source but including it in the PyPI / Conda distros
- Use nodejs-wheel as an optional dependency to build at run time if dist files aren't found?
Consider rewriting JS to drop jQuery
Consider setting up a suite of playwright tests for CI

…tstrap 5.3

…lute pre-commit See twbs/bootstrap#40962 - can hopefully remove these again in a future version of Bootstrap

…accessible The JavaScript bundler (Vite) encapsulates variables in module scope, making them inaccessible to external scripts like module-specific JavaScript files (e.g., multiqc_fastqc.js). This caused 'callAfterDecompressed is not defined' errors. Fixed by explicitly assigning these variables to the window object: - window.callAfterDecompressed (used by module JS files to register callbacks) - window.notEmptyObj (utility function used across modules) - window.mqc_plots (global plot data accessed by multiple files) This ensures backward compatibility with module JavaScript files that expect these as global variables.

multiqc/templates/default/src/js/multiqc.js

multiqc/templates/default/src/js/toolbox/hide.js

multiqc/templates/default/src/js/toolbox/highlights.js

multiqc/templates/default/src/js/toolbox/rename.js

multiqc/templates/default/src/js/toolbox/save-load.js

…ss names in main MultiQC

Resolved merge conflicts in: - multiqc/modules/fastqc/fastqc.py: Combined status_checks logic from main with bootstrap's series_label - multiqc/plots/plot.py: Added _get_series_label function from main - multiqc/plots/violin.py: Used bootstrap's icon and tooltip syntax for warning messages - multiqc/templates/default/src/js/ai-helpers.js: Combined reasoning model functions from main with bootstrap's window scope - multiqc/templates/default/src/js/plots/box.js: Combined BoxPlot class with sorting event listeners - multiqc/templates/default/toolbox.html: Kept bootstrap's modular template includes structure

See https://stackoverflow.com/questions/78708497/prettier-plugins-not-found-with-pre-commit

docs/markdown/development/modules.md

docs/markdown/development/templates.md

ewels · 2025-10-22T17:47:11Z

@multiqc-bot fix linting

claude · 2025-10-22T17:52:29Z

PR Review: Bootstrap Upgrade

I've completed a comprehensive review of this PR. Overall, this is a high-quality modernization effort with good architectural decisions.

Strengths

Code Quality & Best Practices:

Excellent documentation for new features (status bars, Material Design Icons)
Proper use of type hints throughout
Modern Python 3.9+ standards with f-strings
Clean separation with status bar logic in private methods

Architecture:

Vite integration is solid for modern JS/CSS bundling
Material Design Icons properly shared via template functions
Configuration flexibility for status bars (global/per-module/section)
Backwards compatible with opt-in features

Security Concerns

CRITICAL: Potential XSS vulnerability (multiqc/base_module.py:574-611)

The _generate_status_bar_html method generates HTML with unescaped user data:

Issues:

self.anchor and section_anchor are not HTML-escaped before injection
Template renders with {{ s['status_bar_html'] }} which disables Jinja2 autoescape

Risk: Malicious input in anchors could lead to XSS.

Recommendation: Use html.escape() on all anchor values before injecting into HTML.

Potential Bugs

Pre-commit hook (scripts/build-assets.sh): No error handling if cd or npm run build fails
Material icon caching: Unbounded cache growth (minor, should be fine in practice)

Performance

Positive:

SVG icon caching is excellent
JSON serialization is efficient
O(1) complexity for progress bar generation

Test Coverage

Concerns:

No new tests for status bar functionality
No tests for _generate_status_bar_html() XSS protection
No tests for Material Design Icons utility

Recommendation: Add unit tests for base_module.py:518-611

Recommendations Summary

Must Fix (Security):

Escape HTML in _generate_status_bar_html() to prevent XSS
Consider enabling Jinja2 autoescape globally

Should Fix:
3. Add error handling to scripts/build-assets.sh
4. Add unit tests for new functionality

Nice to Have:
5. Use Jinja2 templates for HTML generation instead of f-strings
6. Document security model for HTML injection

Conclusion

This is a well-architected PR that successfully modernizes the MultiQC frontend. Code quality is high and documentation is excellent. Main concern is the potential XSS vulnerability which should be addressed before merging.

Overall Rating: 4/5 stars - Would be 5/5 with XSS fix and better test coverage.

Great work on this modernization effort!

ewels added 28 commits July 3, 2025 14:26

Initial structural work to use SCSS for CSS compilation, with new Boo…

f64ad9f

…tstrap 5.3

Silence depreciation warnings in the CSS compilation so as not to pol…

2efb6e4

…lute pre-commit See twbs/bootstrap#40962 - can hopefully remove these again in a future version of Bootstrap

Initial structural work for bootstrap5 JS - use Vite for build

1a3775e

First stab at converting BS3 -> BS5 for HTML in default template

eeb2b80

Document config.development

40d17d7

Updates for other templates that use default as a base

0f883f1

Update CLI docs for --development now that vite minimises

94b29f6

Make common JS functions global scope

c9083ad

Add file for bootstrap variables. Overwrite table background colour.

7c32a1e

Get the toolbox to show properly, use md icons instead of emoji

44c4844

Start converting hardcoded hex values in custom.css to BS CSS variables

ccb668a

Minor cleanup

e3275cc

Smaller base font size. Outline buttons instead of secondary.

5d58a61

Update how SVG icons are handled, remove all traces of glyphicons

03b65d8

Fix scoping errors in JS for AI summaries

92c0fcd

Fix buttons that launch modal windows

aa9261a

New icon for violin plots

9e293ab

Improve pre-commit build step

8914234

Fix old BS3 form-inline forms

b8473dd

Start work on toolbox: use offcanvas and BS list group

cd93db7

Refactor: Split toolbox javascript into multiple files

c86b893

Prettier: also format jinja template files

33a2ab8

Prettier formatting

30b6626

Fix prettier error

36d256d

Initial effort: dark mode toggle

c9fdef0

Replace toast JS library with native Bootstrap toasts

84c9a08

Nav: make version a link, go to top of page

0eba94f

ewels linked an issue Jul 5, 2025 that may be closed by this pull request

Update template to latest Bootstrap (with dark mode) #1698

Closed

github-advanced-security bot found potential problems Jul 5, 2025

View reviewed changes

ewels added 19 commits October 21, 2025 01:15

Don't import unused bootstrap SCSS

fb002ee

Add description of plotly custom bundle build

8956459

Remove typo $ symbol

a28e308

Several small dark mode fixes

a15b1cc

Mixture of module-specific colour fixes for dark mode

c824ca1

Fix expand / collapse table button

a12e26f

Disco: remove SCSS imports for unused bootstrap components

c01ba31

AI summaries: format highlighted text even when not a sample ID

dae6738

AI summary highlight colours that work in both themes

effb8d3

Fix 'scroll to top' button

d1d9991

Disco SCSS formatting

a96ac11

Add some custom CSS to 'original' to try to cater for Bootstrap 5 cla…

4c03617

…ss names in main MultiQC

Revert changes in templates that inherit from 'original'

7369077

Add a .gitattributes to mark generated files

61930c6

Run pre-commit / prettier on all files

c620d5e

Try to fix CI tests

f9cec0f

Add prettier itself to the hook dependencies

cca4141

Pin prettier dep version

8eef397

ewels marked this pull request as ready for review October 22, 2025 09:22

ewels removed the WIP Work in progress label Oct 22, 2025

Try fixing CI error with pre-commit jinja plugin

a542b85

See https://stackoverflow.com/questions/78708497/prettier-plugins-not-found-with-pre-commit

ewels commented Oct 22, 2025

View reviewed changes

Docs tweaks

750ec90

Ok fine maybe that does need single quotes after all?

b354619

ewels merged commit 22d2199 into main Oct 22, 2025
14 checks passed

ewels deleted the bootstrap-upgrade branch October 22, 2025 17:51

ewels added this to the v1.32 milestone Oct 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bootstrap upgrade#3264

Bootstrap upgrade#3264
ewels merged 111 commits intomainfrom
bootstrap-upgrade

ewels commented Jul 5, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ewels commented Oct 22, 2025

Uh oh!

Uh oh!

claude bot commented Oct 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ewels commented Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To-do

wontfix - push to another day

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ewels commented Oct 22, 2025

Uh oh!

Uh oh!

claude bot commented Oct 22, 2025

PR Review: Bootstrap Upgrade

Strengths

Security Concerns

CRITICAL: Potential XSS vulnerability (multiqc/base_module.py:574-611)

Potential Bugs

Performance

Test Coverage

Recommendations Summary

Conclusion

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ewels commented Jul 5, 2025 •

edited

Loading