Move to packaged Bootstrap #896
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 893-ionicons-web-components #896 +/- ##
===============================================================
+ Coverage 47.79% 47.87% +0.07%
===============================================================
Files 351 351
Lines 11292 11292
Branches 1889 1889
===============================================================
+ Hits 5397 5406 +9
+ Misses 5702 5694 -8
+ Partials 193 192 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
slifty
force-pushed
the
893-ionicons-web-components
branch
from
f28de58 to
be10eb6
Compare
slifty
force-pushed
the
895-package-bootstrap
branch
from
5697c84 to
dd5550f
Compare
slifty
force-pushed
the
893-ionicons-web-components
branch
from
be10eb6 to
e30817f
Compare
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
slifty
force-pushed
the
895-package-bootstrap
branch
from
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.
| "@types/openseadragon": "^4.1.0", | ||
| "angular-shepherd": "^20.0.0", | ||
| "binaryjs": "0.2.1", | ||
| "bootstrap": "^4.1.1", |
There was a problem hiding this comment.
We also have ng-bootstrap apparently...
It seems ng-bootstrap 19.x is designed for Bootstrap 5, not Bootstrap 4.
Also, if we want to move to angular 21, it seems we might be needing ng-bootstrap 20.(the change is already present in the angular 21 PR)
Maybe it's worth having a look at this? https://www.npmjs.com/package/@ng-bootstrap/ng-bootstrap
| @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) |
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 adds a new dependency, so you'll need to run docker compose build web-app 🚨 🚨
This PR should not be merged until #894 is merged.
Resolves #895
Related to #127