Bug 9294

Summary: Move JavaScript out of circulation template
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: enhancement    
Priority: P5 - low CC: campbellreidtait, fridolin.somers, gmcharlt, jonathan.druart, kyle, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 9283    
Bug Blocks: 10884    
Attachments: Bug 9294 - Move JavaScript out of circulation template
Bug 9294 - Move JavaScript out of circulation template
Bug 9294 - Move JavaScript out of circulation template
Bug 9294: Followup FIX csv choice for export
Bug 9294 [Follow-up] Move JavaScript out of circulation template
Bug 9294 - Move JavaScript out of circulation template
SIGNED OFF: Bug-9294-Move-JavaScript-out-of-circulation-template.patch
Bug 9294 - Move JavaScript out of circulation template
Bug 9294 - Move JavaScript out of circulation template
Bug 9294: Followup : add missing lines.

Description Owen Leonard 2012-12-14 20:27:42 UTC
I would like to start working harder to keep JavaScript separated from the templates. I hope that in doing so we can have JavaScript which is more easily cached by the browser, more testable, and potentially able to be minified and/or concatenated with other JS files.

To that end I have begun moving JavaScript out of the circulation template into a page-specific file, circulation.js. I hope that further improvements can be made, perhaps by moving some of the template variables required by the template into HTML5 data- attributes.
Comment 1 Owen Leonard 2012-12-14 20:42:45 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2012-12-14 20:44:33 UTC
Sorry, meant to point out: This patch depends on the one for Bug 9283.
Comment 3 Kyle M Hall 2012-12-21 17:05:08 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2012-12-26 15:09:36 UTC
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
Comment 5 Jonathan Druart 2012-12-26 15:15:03 UTC
Oops sorry, this bug depends on Bug 9283
Comment 6 Jonathan Druart 2012-12-26 15:26:24 UTC
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!
Comment 7 Jonathan Druart 2013-01-04 13:17:41 UTC
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
Comment 8 Owen Leonard 2013-01-04 14:11:05 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2013-01-04 14:12:24 UTC
Fixed conflict with Bug 9289. Still waiting on Bug 9283, on which this patch depends.
Comment 10 Jonathan Druart 2013-03-14 13:44:37 UTC
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.
Comment 11 Jonathan Druart 2013-03-14 13:47:20 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2013-03-14 13:48:09 UTC
I think this quick followup fixes the issue, could you confirm?
Comment 13 Owen Leonard 2013-03-18 18:26:45 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2013-03-19 08:46:42 UTC
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?
Comment 15 Jonathan Druart 2013-03-19 08:53:54 UTC
(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).
Comment 16 Owen Leonard 2013-08-15 20:23:35 UTC Comment hidden (obsolete)
Comment 17 Campbell Reid-Tait 2013-08-28 04:12:11 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2013-09-13 13:29:42 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2013-09-13 13:30:40 UTC
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.
Comment 20 Jonathan Druart 2013-09-13 13:35:57 UTC
(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.
Comment 21 Jonathan Druart 2013-09-13 13:56:02 UTC
Created attachment 21092 [details] [review]
Bug 9294: Followup : add missing lines.
Comment 22 Jonathan Druart 2013-09-13 13:57:09 UTC
Found, 2 lines were missing in the js file.
Now we have the same behavior as master.
Comment 23 Galen Charlton 2013-09-18 16:27:04 UTC
Pushed to master.  Thanks, Owen!
Comment 24 Tomás Cohen Arazi 2013-12-03 18:06:58 UTC
This patch has been pushed to 3.12.x, will be in 3.12.8. It is needed for 10884.

Thanks Owen!