-
Notifications
You must be signed in to change notification settings - Fork 106
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
Use esbuild instead of webpacker #1957
Changes from all commits
73c7049
9dc8c01
4d9d0c4
d189f03
b806053
4e23daf
8d92797
b5ee106
46c72a1
d75f3f8
f8a0a5a
2ec3565
86c2846
8d693c1
e675373
c23884f
ac7a8ba
96e08c5
920f2c1
450a5e1
1f26784
961e953
bb5ac51
cb2fb4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
|
||
|
||
//= link_tree ../images | ||
//= link_tree ../builds |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ | |
// a relevant structure within app/javascript and only use these pack files to reference | ||
// that code so it'll be compiled. | ||
// | ||
// To reference this file, add <%= javascript_pack_tag 'application' %> to the appropriate | ||
// To reference this file, add <%= javascript_include_tag 'application' %> to the appropriate | ||
// layout file, like app/views/layouts/application.html.erb | ||
// | ||
// Uncomment to copy all static images under ../images to the output folder and reference | ||
|
@@ -14,32 +14,35 @@ | |
// const images = require.context('../images', true) | ||
// const imagePath = (name) => images(name, true) | ||
|
||
import 'jquery'; | ||
import 'jquery-ujs' | ||
|
||
// lot's of inline scripts and stuff rely on jquery just being available | ||
window.jQuery = jQuery; | ||
window.$ = $; | ||
import jQuery from 'jquery'; | ||
import 'jquery-ujs'; | ||
import datatables from 'datatables.net'; | ||
import datatablesBs4 from 'datatables.net-bs4/js/dataTables.bootstrap4'; | ||
import datatablesSelect from 'datatables.net-select'; | ||
|
||
import Rails from '@rails/ujs'; | ||
Rails.start(); | ||
|
||
// Import popper.js for Bootstrap 4 | ||
import 'popper.js' | ||
import 'popper.js'; | ||
|
||
// Import Bootstrap 4 | ||
import 'bootstrap/dist/js/bootstrap' | ||
import 'bootstrap/dist/js/bootstrap'; | ||
|
||
// FIXME: confim modals don't work in esbuild. | ||
// import 'data-confirm-modal'; | ||
Comment on lines
+31
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may actually be OK because it's buggy: #665. |
||
|
||
// confim modals | ||
import 'data-confirm-modal'; | ||
import { setNavbarColor } from './config'; | ||
|
||
// Import application stylesheets | ||
import '../stylesheets/application' | ||
// lot's of inline scripts and stuff rely on jquery just being available | ||
window.jQuery = jQuery; | ||
window.$ = jQuery; | ||
|
||
import '../images/OpenOnDemand_powered_by_RGB.svg'; | ||
import '../images/OpenOnDemand_stack_RGB.svg'; | ||
// these plugins don't automatically load, so this loads them. | ||
datatables(window, jQuery); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. more coupling? Now, we're having to pass window and jQuery to the datatable. Seems counter-productive to the goal of decoupling? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This deserves a comment which I'll give it, but this is here to initialize these plugins. They don't directly initialize just by being imported (because of javascript toolchain compilations and how old it is and how it's being invoked). So I had to initialize them to tie them to this jquery otherwise they never invoke. |
||
datatablesBs4(window, jQuery); | ||
datatablesSelect(window, jQuery); | ||
|
||
import { setNavbarColor } from './config'; | ||
Rails.start(); | ||
|
||
jQuery(function(){ | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,3 @@ | ||
import 'datatables.net'; | ||
import 'datatables.net-bs4/js/dataTables.bootstrap4'; | ||
import 'datatables.net-select'; | ||
import 'datatables.net-select-bs4'; | ||
import Handlebars from 'handlebars'; | ||
import {EVENTNAME as SWAL_EVENTNAME} from './sweet_alert.js'; | ||
|
||
|
@@ -198,11 +194,6 @@ class DataTable { | |
data: null, | ||
orderable: false, | ||
defaultContent: '<input type="checkbox">', | ||
render: function (data, type, row, meta) { | ||
var api = new $.fn.dataTable.Api(meta.settings); | ||
let selected = api.rows(meta.row, { selected: true }).count() > 0; | ||
return `<input type="checkbox" ${selected ? 'checked' : ''}> ${selected ? 'checked' : ''}`; | ||
} | ||
Comment on lines
-201
to
-205
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe this is ever used or is useful. plus it adds the text There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. files app test fine with this removed, so we should be good. |
||
}, | ||
{ data: 'type', render: (data, type, row, meta) => data == 'd' ? '<span title="directory" class="fa fa-folder" style="color: gold"><span class="sr-only"> dir</span></span>' : '<span title="file" class="fa fa-file" style="color: lightgrey"><span class="sr-only"> file</span></span>' }, // type | ||
{ name: 'name', data: 'name', className: 'text-break', render: (data, type, row, meta) => `<a class="${row.type} name ${row.type == 'd' ? '' : 'view-file'}" href="${row.url}">${Handlebars.escapeExpression(data)}</a>` }, // name | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're trying to decouple. Why move the datatables here? if there is a piece of functionality that needs datatable access, shouldn't we do the same as we did in the files app?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another thought, by adding datatables import here, we're loading datatables even on pages that do not use datatables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, but jquery plugins don't play nice when jquery has a conflict (i.e., some other definition of jquery). #1846 is an example of this. Indeed - tooltip is a global jquery plugin. Along with poppers that we have installed everywhere.
I figure if we buy in on jquery, we buy in on it's plugins too.
I can try to fix this but it didn't work easily so i'll bet we have to use jquery's noConflict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to fix. I was just asking the question.