Move to packaged Bootstrap - #896
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #896 +/- ##
==========================================
+ Coverage 47.83% 47.89% +0.05%
==========================================
Files 351 351
Lines 11306 11311 +5
Branches 1892 1893 +1
==========================================
+ Hits 5408 5417 +9
+ Misses 5709 5701 -8
- Partials 189 193 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
f28de58 to
be10eb6
Compare
5697c84 to
dd5550f
Compare
be10eb6 to
e30817f
Compare
dd5550f to
83f9ced
Compare
aasandei-vsp
left a comment
aasandei-vsp
left a comment
There was a problem hiding this comment.
It seems we might be needing to figure out multiple dependencies, not only the bootstrap one.
| @import 'bootstrap/scss/type'; | ||
| @import 'bootstrap/scss/tables'; | ||
| @import 'bootstrap/scss/forms'; | ||
| @import 'bootstrap/scss/buttons'; |
There was a problem hiding this comment.
This one might have some issues, I noticed on 127-remove-vendor branch that the upload button orange background is not showing.
And another one, seems to be for orange buttons?
|
Also, the merge button is green and I do not think that is correct, right? |
Just because this PR is currently merging into a branch (not main) |
83f9ced to
cf0c007
Compare
|
@aasandei-vsp I've added a second commit which upgrades to bootstrap 5. I attempted to go through the screens to ensure parity to production! Note: the new version means you'll have to do the "rebuild the image" dance unfortunately. |
|
Removing myself as a reviewer since you two have this under control. |
aasandei-vsp
left a comment
aasandei-vsp
left a comment
There was a problem hiding this comment.
Built it locally -- OK
Tested a few of the components -- OK
LGTM
We had included Bootstrap v4 as vendored scss instead of loading it via npm / packages. This isn't a great practice for a couple of reasons, not the least of which being the fact that we can't benefit from things like dependabot and dependency tree checks. This intentionally does not migrate to Bootstrap v5, but simply changes how v4 is being installed. Issue #895 Remove vendored bootstrap
We recently transitioned from a vendor bootstrap (with source checked in) to an npm managed version. This exposed the fact that our ng-bootstrap is built to require bootstrap 5, but we were still using bootstrap 4. This migrates our UX to the latest Bootstrap with an attempt to have minimal-to-no actual impact on the UX.
64fb465 to
ae51380
Compare
This upgrade comes with a lot of package bumps, as expected. For the short term we have to override ng-mocks' peer dependencies. There is an open PR to add formal angular support. If tests pass, this is OK for now. We also need to use a release candidate for angular firebase. This is not ideal, but similarly if it works that's OK. It should be noted that angular firebase is not maintained at the cadence we would prefer and so this is another reason it's good we're moving away from it. Issue #869 Upgrade to Angular 21
Update to Angular 21
3 participants
This PR removes our "vendor" setup for Bootstrap and moves us to using an npm package instead. This will make it easier to avoid version drift and also allows us to remove third party code from our repository.
This also updates our version of Bootstrap to v5, which is the latest version and is compatible with our other angular dependencies.
🚨 🚨 This adds a new dependency, so you'll need to run docker compose build web-app 🚨 🚨
Resolves #895
Related to #127