Description
Owen Leonard
2012-12-14 20:27:42 UTC
Created attachment 14154 [details] [review] Bug 9294 - Move JavaScript out of circulation template This patch creates a new js file, js/pages/circulation.js, and begins the process of moving JavaScript into it from circulation.tt. I have started the process by focusing on sections which are least dependent on template variables. To test, load a patron for checkout who has something checked out. There should be no JavaScript errors: - Tabs should work. - Showing and hiding the "Add message" form should work. - The "specify due date" datepicker should work. - Controls for selecting checkboxes in the checkouts table should work - With checkout exports enabled, warnings should be displayed if you have no checkouts selected, or if you try to export as CSV and have no CSV profile specified in your ExportWithCsvProfile. Sorry, meant to point out: This patch depends on the one for Bug 9283. Created attachment 14251 [details] [review] Bug 9294 - Move JavaScript out of circulation template This patch creates a new js file, js/pages/circulation.js, and begins the process of moving JavaScript into it from circulation.tt. I have started the process by focusing on sections which are least dependent on template variables. To test, load a patron for checkout who has something checked out. There should be no JavaScript errors: - Tabs should work. - Showing and hiding the "Add message" form should work. - The "specify due date" datepicker should work. - Controls for selecting checkboxes in the checkouts table should work - With checkout exports enabled, warnings should be displayed if you have no checkouts selected, or if you try to export as CSV and have no CSV profile specified in your ExportWithCsvProfile. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt I prefer to wait before passing qa on this patch. It depends on another one not already pushed and it interferes with Bug 9289. On rebasing, we won't forget to uncomment this line! CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Created attachment 14417 [details] [review] Bug 9294 - Move JavaScript out of circulation template This patch creates a new js file, js/pages/circulation.js, and begins the process of moving JavaScript into it from circulation.tt. I have started the process by focusing on sections which are least dependent on template variables. To test, load a patron for checkout who has something checked out. There should be no JavaScript errors: - Tabs should work. - Showing and hiding the "Add message" form should work. - The "specify due date" datepicker should work. - Controls for selecting checkboxes in the checkouts table should work - With checkout exports enabled, warnings should be displayed if you have no checkouts selected, or if you try to export as CSV and have no CSV profile specified in your ExportWithCsvProfile. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> QA comment: If ExportRemoveFields is filled and no csv profile defined in ExportWithCsvProfile, the csv choice appears in the dropdown list but does not work. I think we should remove it from the list if no profile is defined. Marked as Failed QA. Created attachment 16127 [details] [review] Bug 9294: Followup FIX csv choice for export If ExportRemoveFields is filled and no csv profile defined in ExportWithCsvProfile, the csv choice appears in the dropdown list but does not work. I think this quick followup fixes the issue, could you confirm? Created attachment 16303 [details] [review] Bug 9294 [Follow-up] Move JavaScript out of circulation template This patch corrects a JavaScript error in the original patch caused by an incorrectly named variable. Instead of correcting the name I'm removing the error-handling around missing CSV profiles alltogether in favor of Jonathan's suggestion to simply hide the CSV option if no formats are specified. To test this follow-up perform exports of checkouts with ExportRemoveFields filled and with ExportWithCsvProfile both empty and not empty. Owen, the first patch does not apply fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt). Could your rebase it please? (In reply to comment #14) > Owen, the first patch does not apply > fatal: sha1 information is lacking or useless > (koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt). > Could your rebase it please? In fact it is the second one! fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/js/pages/circulation.js). Created attachment 20382 [details] [review] Bug 9294 - Move JavaScript out of circulation template This patch creates a new js file, js/pages/circulation.js, and begins the process of moving JavaScript into it from circulation.tt. I have started the process by focusing on sections which are least dependent on template variables. To test, load a patron for checkout who has something checked out. There should be no JavaScript errors: - Tabs should work. - Showing and hiding the "Add message" form should work. - The "specify due date" datepicker should work. - Controls for selecting checkboxes in the checkouts table should work Created attachment 20684 [details] [review] SIGNED OFF: Bug-9294-Move-JavaScript-out-of-circulation-template.patch Bug 9294 - Move JavaScript out of circulation template (14.29 KB, patch) 2013-01-04 14:11 UTC, Owen Leonard and Bug 9294 [Follow-up] Move JavaScript out of circulation template (3.16 KB, patch) 2013-03-18 18:26 UTC, Owen Leonard were unable to be applied error fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 9294 - Move JavaScript out of circulation template Created attachment 21090 [details] [review] Bug 9294 - Move JavaScript out of circulation template This patch creates a new js file, js/pages/circulation.js, and begins the process of moving JavaScript into it from circulation.tt. I have started the process by focusing on sections which are least dependent on template variables. To test, load a patron for checkout who has something checked out. There should be no JavaScript errors: - Tabs should work. - Showing and hiding the "Add message" form should work. - The "specify due date" datepicker should work. - Controls for selecting checkboxes in the checkouts table should work Signed-off-by: Campbell Reid-Tait <campbellreidtait@gmail.com> Created attachment 21091 [details] [review] Bug 9294 - Move JavaScript out of circulation template This patch creates a new js file, js/pages/circulation.js, and begins the process of moving JavaScript into it from circulation.tt. I have started the process by focusing on sections which are least dependent on template variables. To test, load a patron for checkout who has something checked out. There should be no JavaScript errors: - Tabs should work. - Showing and hiding the "Add message" form should work. - The "specify due date" datepicker should work. - Controls for selecting checkboxes in the checkouts table should work Signed-off-by: Campbell Reid-Tait <campbellreidtait@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Just found a weird behavior with checkboxes in the checkouts table, but still exists on master. (In reply to Jonathan Druart from comment #19) > Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> > Just found a weird behavior with checkboxes in the checkouts table, but > still exists on master. Arg! It is weird on master, but much more with this patch. With this patch, checkboxes can be all checked together. Failed QA. Created attachment 21092 [details] [review] Bug 9294: Followup : add missing lines. Found, 2 lines were missing in the js file. Now we have the same behavior as master. Pushed to master. Thanks, Owen! This patch has been pushed to 3.12.x, will be in 3.12.8. It is needed for 10884. Thanks Owen! |