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.
Created attachment 23039 [details] [review] Bug 11270: Refactor hit highliginting code This patch tries to factor out some of the hit highlighting code to aid in future maintainability. It should not affect current operation.
Looks like this should be "Needs signoff"
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 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?
(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
I could not apply the patch. Can you please check. Plus needs prog removal
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.
Created attachment 38940 [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.
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.
Changing back to assigned to Martin, since he posted the initial patches, and he never responded with a test plan. :)
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
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