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

Upgrade bootstrap and associated dependencies, fixes #2572 #2580

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

xmunoz
Copy link
Member

@xmunoz xmunoz commented May 11, 2022

I rule and have completed some work on Case Manager that's ready for review!

This pull request makes the following changes:

  • Upgrade bootstrap and its dependencies so we stop getting warnings on yarn install

Docs

Notes

Building webpack is throwing lots of deprecation warnings about stuff originating in bootstrap, and therefore nothing we should worry about. They look like this:

webpacker_1  | DEPRECATION WARNING: Using / for division outside of calc() is deprecated and will be removed in Dart Sass 2.0.0.
webpacker_1  | 
webpacker_1  | Recommendation: math.div($grid-gutter-width, 2) or calc($grid-gutter-width / 2)
webpacker_1  | 
webpacker_1  | More info and automated migrator: https://sass-lang.com/d/slash-div
webpacker_1  | 
webpacker_1  |     ╷
webpacker_1  | 353 │ $container-padding-x: $grid-gutter-width / 2 !default;
webpacker_1  |     │                       ^^^^^^^^^^^^^^^^^^^^^^
webpacker_1  |     ╵
webpacker_1  |     node_modules/bootstrap/scss/_variables.scss 353:23  @import
webpacker_1  |     node_modules/bootstrap/scss/bootstrap.scss 11:9     @import
webpacker_1  |     app/javascript/styles/bootstrap.scss 22:9           @import
webpacker_1  |     app/javascript/packs/application.scss 6:9           root stylesheet

@xmunoz xmunoz marked this pull request as draft May 11, 2022 20:07
@xmunoz xmunoz force-pushed the upgrade-bootstrap branch 2 times, most recently from a85874a to 3fb9fcc Compare May 11, 2022 20:49
@xmunoz
Copy link
Member Author

xmunoz commented May 11, 2022

Not sure why the bin/rails assets:precompile is failing. Will probably need help from someone else to get past that. On a positive note, this works locally, with only a small number of noticeable UI bugs.

@xmunoz
Copy link
Member Author

xmunoz commented Jul 7, 2022

Awesome, with the webpacker exorcism, it looks like asset compilation is now happening successfully. Just need to finish the bootstrap migration, which should hopefully fix the failing tests.

@xmunoz
Copy link
Member Author

xmunoz commented Jul 7, 2022

Alright, this is where we are right now:

Finished in 386.277882s, 1.7241 runs/s, 4.1317 assertions/s.
666 runs, 1596 assertions, 24 failures, 46 errors, 0 skips
Coverage report generated for Unit Tests to /home/runner/work/dcaf_case_management/dcaf_case_management/coverage. 1191 / 1649 LOC (72.23%) covered.
Error: Process completed with exit code 1.

@xmunoz xmunoz force-pushed the upgrade-bootstrap branch 2 times, most recently from b446265 to 2d1d77b Compare July 12, 2022 19:01
@xmunoz
Copy link
Member Author

xmunoz commented Jul 28, 2022

The main issues, as far as I can tell, is that the bootstrap javascript for menus (i.e. popper.js) is not being loaded properly. I don't know why not, since it's been explicitly added as a dependency in package.json and loaded in app/javascript/application.js, but the menu dropdowns are still not working. There are also a smaller number of stylebugs, but by far the biggest question is why isn't this JS being loaded.

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.

None yet

2 participants