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

Move js require higher in the install generator #840

Closed
wants to merge 1 commit into from

Conversation

kirkkwang
Copy link
Contributor

@kirkkwang kirkkwang commented Jul 20, 2023

Summary

In order to make Bulkrax's javascript work in Hydra, we needed to add bootstrap's JS to bulkrax, along with jquery since bootstrap depends on jquery.
In hyku apps, this caused unintended behavior with the dataTables in the dashboard.
The data tables needed to be required AFTER bulkrax in order to keep their functionality in hyku.

This commit will alter the install generator to insert the require for bulkrax/application above dataTables require.

co-author: @summer-cook

Ref:

Screenshots

generated require image

Note

This may need to be changed for existing hyku applications with Bulkrax.
adding bootstrap javascript was a potential breaking change for some applications: existing hyku applications will need to change the order of these requires.

This commit will alter the install generator to move the require for
bulkrax/application above dataTables require.  This was causing jquery
errors in Hyku applications because of recent changes.

Ref:
  - #836
@kirkkwang kirkkwang added the minor-ver for release notes label Jul 20, 2023
@kirkkwang kirkkwang marked this pull request as ready for review July 20, 2023 20:58
@kirkkwang
Copy link
Contributor Author

Instead of tinkering the generator, we are opting to adjust Hyda's application.js to include bootstrap instead of having it in Bulkrax

@kirkkwang kirkkwang closed this Jul 24, 2023
@kirkkwang kirkkwang deleted the move-js-require-higher branch July 24, 2023 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant