Bug 11035 - Replace shortcut plugin jquery.hotkeys.min.js
Summary: Replace shortcut plugin jquery.hotkeys.min.js
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Blou
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-10 15:51 UTC by Blou
Modified: 2015-06-04 23:23 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11035 - (first patch) Replace shortcut plugin jquery.hotkeys.min.js (18.25 KB, patch)
2013-10-10 17:52 UTC, Blou
Details | Diff | Splinter Review
Bug 11035 - (patching the patch) Replace shortcut plugin jquery.hotkeys.min.js (1.20 KB, patch)
2013-10-23 12:38 UTC, Blou
Details | Diff | Splinter Review
Bug 11035 - Replace shortcut plugin jquery.hotkeys.min.js (18.10 KB, patch)
2013-10-25 15:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11035 - Replace shortcut plugin jquery.hotkeys.min.js (1.25 KB, patch)
2013-10-25 15:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11035 - Replace shortcut plugin jquery.hotkeys.min.js (18.58 KB, patch)
2013-12-03 11:59 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11035 - Replace shortcut plugin jquery.hotkeys.min.js (1.32 KB, patch)
2013-12-03 11:59 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Blou 2013-10-10 15:51:41 UTC
(Not sure I have have the right Component or Severity, feedbacks welcomed).

A sponsor asked for some specific keyboard shortcuts.  After discussion a month ago with (oleonard, jcamins), it seemed a change of plugin was needed because the current one is not maintained and would limit our ability to upgrade jquery, simply.

I was suggested a few independant ones, and implemented my feature with openjs's.

This bug is ONLY about changing the plugin.  It brings no new functionnality nor does it changes behavior.  Enhancement to shortcut could bring a lot of discussion.

Refression TESTING: 
1) apply the patch, 
2) in intranet, validate that the three available shortcuts (Alt-r, alt-u, alt-q) are still functionning perfectly.
Comment 1 Blou 2013-10-10 17:52:37 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-10-23 08:53:43 UTC
Note for QA: circ/offline-mf.tt uses jquery.hotkeys.min.js too.
Comment 3 Blou 2013-10-23 12:38:35 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2013-10-25 15:39:07 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2013-10-25 15:39:15 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2013-11-28 14:34:53 UTC
(In reply to Blou from comment #3)
> Seems a new file circ/offline-mf.tt got added with hotkeys.js.  I replaced
> it but I couldn't find ANY usage of it to fix.

Same for me. Jared, could you confirm your development for Offline circulation (bug 10240) includes this js file but don't use it?
Comment 7 Jared Camins-Esakov 2013-11-28 14:38:58 UTC
(In reply to Jonathan Druart from comment #6)
> (In reply to Blou from comment #3)
> > Seems a new file circ/offline-mf.tt got added with hotkeys.js.  I replaced
> > it but I couldn't find ANY usage of it to fix.
> 
> Same for me. Jared, could you confirm your development for Offline
> circulation (bug 10240) includes this js file but don't use it?

The offline circulation module includes the file via doc-head-close.inc. If the file is changed there, it must be changed in the manifest as well.
Comment 8 Jonathan Druart 2013-11-28 14:57:19 UTC
Haaa :)
Thanks Jared for the clarification!
Comment 9 Katrin Fischer 2013-12-03 11:59:05 UTC
Created attachment 23259 [details] [review]
[PASSED QA] Bug 11035 - Replace shortcut plugin jquery.hotkeys.min.js

To old plugin prevents the upgrade to the latest jquery.  By replacing with openjs' shortcut.js, we have now one less hurdle in upgrading Koha to latest.

Changes:
- removed jquery.hotkeys.min.js
- added shortcut.js
- modified the related includes (doc-head-close.inc and help-top.inc)
- modified the calls in staff-global.js

Regression test:
1) apply the patch
2) in the intranet, test the shortcuts alt-q, alt-r, alt-u.

These are the only affected functionalities.  There is no new functionnality.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script. Tested existing
shortcuts for the search bar tabs on various circulation
related pages.

This uses a Javascript under BSD license, I wonder if it
should be listed in the About>licenses section that we
are using it. As Yui is also BSD I assume the license is
compatible.

+/**
+ * http://www.openjs.com/scripts/events/keyboard_shortcuts/
+ * Version : 2.01.B
+ * By Binny V A
+ * License : BSD
+ */
Comment 10 Katrin Fischer 2013-12-03 11:59:18 UTC
Created attachment 23260 [details] [review]
[PASSED QA] Bug 11035 - Replace shortcut plugin jquery.hotkeys.min.js

Seems a new file circ/offline-mf.tt got added with hotkeys.js.  I replaced it but I couldn't find ANY usage of it to fix.

Rebased and fixed.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 11 Katrin Fischer 2013-12-03 12:00:44 UTC
Comment on attachment 23259 [details] [review]
[PASSED QA] Bug 11035 - Replace shortcut plugin jquery.hotkeys.min.js

Please have different subjects on various patches - or git bz automatically obsoletes the first when attaching the second :)

Also, Galen - please can you take a look at my note on the first patch before pushing?
Comment 12 Galen Charlton 2013-12-03 15:59:35 UTC
Pushed to master, along with followups to move it out of jquery/plugins (as shortcuts.js is not actually a jQuery plugin) and to update the About | Licenses page.

Thanks, Philippe!
Comment 13 Fridolin Somers 2013-12-13 15:29:05 UTC
Those patches has been pushed to 3.14.x, will be in 3.14.1.