Bug 11270

Summary: Search Result highlighting should be refactored
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: SearchingAssignee: Bugs List <koha-bugs>
Status: CLOSED WONTFIX QA Contact: Testopia <testopia>
Severity: trivial    
Priority: P5 - low CC: campbellreidtait, chris, dcook, gaetan.boisson, gitbot, jonathan.druart, kyle, magnus, martin.renvoize, mtompset, paul.poulain, savitra.sirohi, spalding, srdjan
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 6149    
Bug Blocks:    
Attachments: Bug 11270: Refactor hit highliginting code
Bug 11270: Search Result highlighting should be refactored (master)
Bug 11270: Search Result highlighting should be refactored (3.16.x)
Bug 11270: Search Result highlighting should be refactored (3.16.x)

Description Martin Renvoize (ashimema) 2013-11-19 16:18:05 UTC
Whilst working on Bug 6149 I found there to be many repeats of code in different file relating to the search highlighting.

This patch tries to factor out some of the code into include's to enhance maintainability.
Comment 1 Martin Renvoize (ashimema) 2013-11-19 16:20:55 UTC Comment hidden (obsolete)
Comment 2 Magnus Enger 2014-06-16 18:31:37 UTC
Looks like this should be "Needs signoff"
Comment 3 David Cook 2014-06-17 23:53:02 UTC
Could you post a test plan?

Also, the code seems slightly different in opac-results.tt versus opac-results-grouped.tt.

I've never used the latter (as I don't use pazpar2), so I'm not sure I can test this one...
Comment 4 Mark Tompsett 2014-06-18 20:15:09 UTC
Comment on attachment 23039 [details] [review]
Bug 11270: Refactor hit highliginting code

Review of attachment 23039 [details] [review]:
-----------------------------------------------------------------

Please split the bootstrap and prog patches into two seperate patch files. It is my understanding, which could be flawed, but given that 3.18 is likely to deprecate prog/cssr, you want to not include prog/cssr patches in master.

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results-grouped.tt
@@ -356,5 @@
> -		    $("td").highlight(q_array[x]);
> -	    }
> -    }
> -    $(".highlight_toggle").toggle();
> -}

The two functions were in this code, and are moved to opac-highlight.inc, but there is no include in this file. How are these functions still provided?
Comment 5 Chris Cormack 2014-06-18 20:23:43 UTC
(In reply to M. Tompsett from comment #4)
> Comment on attachment 23039 [details] [review] [review]
> Bug 11270: Refactor hit highliginting code
> 
> Review of attachment 23039 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> Please split the bootstrap and prog patches into two seperate patch files.
> It is my understanding, which could be flawed, but given that 3.18 is likely
> to deprecate prog/cssr, you want to not include prog/cssr patches in master.


prog and cssr already are deprecated, but they will be removed in 3.18.

Here is the relevant coding guideline
http://wiki.koha-community.org/wiki/Coding_Guidelines#HTML5:_Deprecation_of_the_.27prog.27_and_.27CCSR.27_OPAC_themes
Comment 6 Srdjan Jankovic 2014-08-15 02:44:17 UTC
I could not apply the patch. Can you please check. Plus needs prog removal
Comment 7 Mark Tompsett 2015-05-07 03:22:20 UTC
Created attachment 38939 [details] [review]
Bug 11270: Search Result highlighting should be refactored (master)

Whilst working on Bug 6149 I found there to be many repeats
of code in different file relating to the search highlighting.

This patch tries to factor out some of the code into include's
to enhance maintainability.

This is just the bootstrap portions. Untested.
This should apply. However, Martin Renvoize has yet to post a
test plan.
Comment 8 Mark Tompsett 2015-05-07 03:59:11 UTC Comment hidden (obsolete)
Comment 9 Mark Tompsett 2015-05-07 04:05:10 UTC
Created attachment 38941 [details] [review]
Bug 11270: Search Result highlighting should be refactored (3.16.x)

Whilst working on Bug 6149 I found there to be many repeats
of code in different file relating to the search highlighting.

This patch tries to factor out some of the code into include's
to enhance maintainability.

This is just the bootstrap portions. Untested.
This should apply. However, Martin Renvoize has yet to post a
test plan.
Comment 10 Mark Tompsett 2015-05-07 04:10:00 UTC
Changing back to assigned to Martin, since he posted the initial patches, and he never responded with a test plan. :)
Comment 11 Gaetan Boisson 2018-03-14 13:43:52 UTC
This has been open for a long time with no solution. I propose to remove the feature from the interface until this is fixed:
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20398
Comment 12 Martin Renvoize (ashimema) 2019-04-10 12:16:03 UTC
I don't have any intentions of coming back to this now.. it was a simple refactor at the time to reduce code duplication..

Marking RESOLVED WONTFIX