Bug 20931

Summary: JS error "ReferenceError: $ is not defined" when CircSidebar is turned on
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: charles.farmer, fridolin.somers, josef.moravec, katrin.fischer, martin.renvoize, nick, oleonard
Version: unspecified   
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: 16530, 19700    
Bug Blocks:    
Attachments: Bug 20931: JS error "ReferenceError: $ is not defined" when CircSidebar is turned on
Bug 20931: JS error "ReferenceError: $ is not defined" when CircSidebar is turned on
Bug 20931: JS error "ReferenceError: $ is not defined" when CircSidebar is turned on

Description Jonathan Druart 2018-06-13 13:01:27 UTC
If CircSidebar is set and you hit /cgi-bin/koha/circ/returns.pl, you will get "ReferenceError: $ is not defined"
Comment 1 Katrin Fischer 2018-06-18 21:42:58 UTC
I can confirm the error, but haven't been able to locate the mistake in the code yet. Only thing I notice is some errors from the validator - 2 stay </div> tags.
Comment 2 Katrin Fischer 2018-06-18 21:44:13 UTC
Ah, I think the JavaScript for navbar needs to be moved to the bottom.
Comment 3 Owen Leonard 2018-06-19 12:28:51 UTC
(In reply to Katrin Fischer from comment #2)
> Ah, I think the JavaScript for navbar needs to be moved to the bottom.

It needs to be moved to a separate JS file or a JS file which is already included on the affected pages. It's on my to-do list but someone else can take it if they want.

As far as I know it's only a cosmetic issue -- the highlighting of the current page won't work.
Comment 4 Owen Leonard 2018-06-20 17:33:05 UTC Comment hidden (obsolete)
Comment 5 Charles Farmer 2018-06-22 21:28:18 UTC
Could reproduce, and went through most pages in the sidebar: the ones I visited were indeed highlighted as active, and the '$' error went away after applying patch.
Comment 6 Charles Farmer 2018-06-22 21:29:52 UTC
Created attachment 76312 [details] [review]
Bug 20931: JS error "ReferenceError: $ is not defined" when CircSidebar is turned on

This patch reorganizes the way sidebar menus get highlighted based on
the current page. A global function is added to handle most cases.
Individual menu-handling scripts have been modified to handle only the
edge cases which aren't covered by the global one.

A new class is added to the global CSS file so that highlighted menu
links can be custom-styled.

To test, apply the patch and clear your cache if necessary.

View various pages to confirm that current-page-highlighting in the
sidebar menu is working correctly and that there are no JS errors in the
browser console. For example:

 - Administration -> Currencies and exchange rates.
 - Acquisitions -> Invoices
 - Tools -> Patron lists
 - Tools -> Export

And with CircSidebar turned on test various circulation pages, e.g.

 - Circulation -> Set library

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Comment 7 Jonathan Druart 2018-06-25 20:10:23 UTC
Created attachment 76416 [details] [review]
Bug 20931: JS error "ReferenceError: $ is not defined" when CircSidebar is turned on

This patch reorganizes the way sidebar menus get highlighted based on
the current page. A global function is added to handle most cases.
Individual menu-handling scripts have been modified to handle only the
edge cases which aren't covered by the global one.

A new class is added to the global CSS file so that highlighted menu
links can be custom-styled.

To test, apply the patch and clear your cache if necessary.

View various pages to confirm that current-page-highlighting in the
sidebar menu is working correctly and that there are no JS errors in the
browser console. For example:

 - Administration -> Currencies and exchange rates.
 - Acquisitions -> Invoices
 - Tools -> Patron lists
 - Tools -> Export

And with CircSidebar turned on test various circulation pages, e.g.

 - Circulation -> Set library

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Nick Clemens 2018-07-02 13:15:05 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Martin Renvoize 2018-07-02 16:58:51 UTC
Pushed to 18.05.x for 18.05.02
Comment 10 Fridolin Somers 2018-07-04 14:07:12 UTC
Depends on Bug 19700 not in 17.11.x