Bug 11270 - Search Result highlighting should be refactored
Summary: Search Result highlighting should be refactored
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords:
Depends on: 6149
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-19 16:18 UTC by Martin Renvoize
Modified: 2020-01-06 20:17 UTC (History)
15 users (show)

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


Attachments
Bug 11270: Refactor hit highliginting code (9.35 KB, patch)
2013-11-19 16:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 11270: Search Result highlighting should be refactored (master) (5.26 KB, patch)
2015-05-07 03:22 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 11270: Search Result highlighting should be refactored (3.16.x) (8.69 KB, patch)
2015-05-07 03:59 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 11270: Search Result highlighting should be refactored (3.16.x) (8.68 KB, patch)
2015-05-07 04:05 UTC, Mark Tompsett
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 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 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