Having a separate file for JavaScript makes the file cache-able and lint-able. This patch will move the <script> block (which contains no template markup) into a separate file.
I'm also moving the contents of table_filters.js into the new JS file (tablesettings.js). table_filters.js contained only two functions and was only used on pages which already include columns_settings.inc.
Created attachment 119024 [details] [review] Bug 28058: Move javascript out of columns_settings.inc into separate file This patch moves the JavaScript contained in a <script> block inside columns_settings.inc into a separate file. The new JavaScript file, tablesettings.js, contains some corrections to minor errors highlighted by eslint (missing semicolons, undeclared variables, etc). I'm also moving the contents of table_filters.js into tablesettings.js and removing table_filters.js. I don't think it necessary to have a whole separate JS file which is only used on pages which already include columns_settings.inc. To test, apply the patch and perform a catalog search in the staff client. - On the bibliographic detail page, confirm that the table of holdings still works correctly: Sorting, search, column filters, and export. - Confirm that clicking the "Activate filters" control activates the filter fields at the top of each table column. - Go to Reports -> Lost items and perform the same tests.
Created attachment 119163 [details] [review] Bug 28058: Move javascript out of columns_settings.inc into separate file This patch moves the JavaScript contained in a <script> block inside columns_settings.inc into a separate file. The new JavaScript file, tablesettings.js, contains some corrections to minor errors highlighted by eslint (missing semicolons, undeclared variables, etc). I'm also moving the contents of table_filters.js into tablesettings.js and removing table_filters.js. I don't think it necessary to have a whole separate JS file which is only used on pages which already include columns_settings.inc. To test, apply the patch and perform a catalog search in the staff client. - On the bibliographic detail page, confirm that the table of holdings still works correctly: Sorting, search, column filters, and export. - Confirm that clicking the "Activate filters" control activates the filter fields at the top of each table column. - Go to Reports -> Lost items and perform the same tests. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 119165 [details] [review] Bug 28058: Move javascript out of columns_settings.inc into separate file This patch moves the JavaScript contained in a <script> block inside columns_settings.inc into a separate file. The new JavaScript file, tablesettings.js, contains some corrections to minor errors highlighted by eslint (missing semicolons, undeclared variables, etc). I'm also moving the contents of table_filters.js into tablesettings.js and removing table_filters.js. I don't think it necessary to have a whole separate JS file which is only used on pages which already include columns_settings.inc. To test, apply the patch and perform a catalog search in the staff client. - On the bibliographic detail page, confirm that the table of holdings still works correctly: Sorting, search, column filters, and export. - Confirm that clicking the "Activate filters" control activates the filter fields at the top of each table column. - Go to Reports -> Lost items and perform the same tests. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Shouldn't we also apply the changes to the OPAC? Shouldn't the file be named table_settings.js instead?
I called it "columns_settings.js" because it corresponds to "columns_settings.inc" and I thought it made sense for them to be the same. I'm happy to change it. I could also change the name of columns_settings.inc at the same time. Anyone have preferences? I can do the same in the OPAC but I think that can happen on another bug without holding up this one.
(In reply to Owen Leonard from comment #6) > I called it "columns_settings.js" because it corresponds to > "columns_settings.inc" and I thought it made sense for them to be the same. > I'm happy to change it. I could also change the name of columns_settings.inc > at the same time. > > Anyone have preferences? Everything should now be "tables_settings". > I can do the same in the OPAC but I think that can happen on another bug > without holding up this one. For consistency I would prefer to keep both in sync. We can do it on a separate bug report but I will push them together.
Created attachment 119321 [details] [review] Bug 28058: (follow-up) Make same change in the OPAC; Use "table_settings" as file name This patch moves the JavaScript contained in a <script> block inside columns_settings.inc in the OPAC into a separate file. The new JavaScript file, table_settings.js, contains some corrections to minor errors highlighted by eslint (missing semicolons, undeclared variables, etc). This patch also moves template include files and corresponding JavaScript files to the name "table_settings" instead of "columns_settings." Updated are all files where "columns_settings.inc" was referenced. To test, apply the patch and test pages in the OPAC and staff interface that use the table_settings include file, e.g. Staff interface: - Administration -> Libraries - Circulation -> Check out OPAC: - Course reserves - Logged in patron -> "Your summary" In all cases DataTable should display correctly, with all associated controls. Changes made to the tables' configuration via 'Administration -> Table settings' should be reflected in the corresponding page.
Patch no longer applies 8-(..