Bug 12296 - OPAC search box should be customisable
Summary: OPAC search box should be customisable
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Robin Sheat
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-21 02:53 UTC by Robin Sheat
Modified: 2015-06-04 23:33 UTC (History)
5 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:
Circulation function:


Attachments
Bug 12296 - make search box customisable with OpacCustomSearch (4.19 KB, patch)
2014-05-22 02:24 UTC, Robin Sheat
Details | Diff | Splinter Review
Bug 12296 - make search box customisable with OpacCustomSearch (19.36 KB, patch)
2014-05-26 03:31 UTC, Robin Sheat
Details | Diff | Splinter Review
Bug 12296 - make search box customisable with OpacCustomSearch (19.36 KB, patch)
2014-06-18 13:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12296 - make search box customisable with OpacCustomSearch (19.65 KB, patch)
2014-06-18 14:35 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 12296 - make search box customisable with OpacCustomSearch (19.33 KB, patch)
2014-06-25 02:44 UTC, Robin Sheat
Details | Diff | Splinter Review
[PASSED QA] Bug 12296 - make search box customisable with OpacCustomSearch (19.49 KB, patch)
2014-06-28 10:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 12296: (QA followup) remove rebasing leftover (972 bytes, patch)
2014-10-18 13:39 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Sheat 2014-05-21 02:53:56 UTC
It should be possible to swap out the search box with custom HTML if that's a thing you need to do, e.g. to integrate other search options. Ideally this'd be able to happen without a template change, just by putting HTML into a system preference.
Comment 1 Katrin Fischer 2014-05-21 09:17:46 UTC
I think it would be nice :)

2 problems that I think need to be taken care of would be translations and selecting the right index after a search was performed - we keem the search term in the search box and the pull down should also be the one you selected before.
Comment 2 Robin Sheat 2014-05-21 22:39:52 UTC
These would be problems, however they're not easy to solve. If they affect your use case, then you'll need to improve on this method :) (once I get my patch together to be submitted.)
Comment 3 Robin Sheat 2014-05-22 02:24:13 UTC Comment hidden (obsolete)
Comment 4 Eugene Espinoza 2014-05-22 03:10:46 UTC
Hi! This seems related to my post:
http://koha.1045719.n5.nabble.com/koha-item-types-dropdown-instead-of-library-dropdown-td5798062.html
If it is, then I would be glad for this. :)
Comment 5 Robin Sheat 2014-05-22 03:15:08 UTC
(In reply to Eugene Espinoza from comment #4)
> Hi! This seems related to my post:
> http://koha.1045719.n5.nabble.com/koha-item-types-dropdown-instead-of-
> library-dropdown-td5798062.html
> If it is, then I would be glad for this. :)

While it could, it probably wouldn't be the best way. It seems that some jQuery is more likely to be problem-free.
Comment 6 Magnus Enger 2014-05-22 07:47:47 UTC
(In reply to Katrin Fischer from comment #1)
> translations

I did some testing on that a while back, can't remember where I commented on it. But if we do a TT-eval on the contents of the syspref:

    [% OpacCustomSearch | eval %]

we can have TT code and logic based on languages in the contents of the syspref:

    [% IF ( lang == 'de' ) %]
        Der alte Zuul
    [% ELSE %]
        Ye olde Zuul
    [% END %]

(I have not tested this again now so please regard this as pseudocode.)

Arguments against:

1. We might not want to do eval on a syspref
2. People would need to know TT to edit the (logic of the) syspref. (If we didn't invent some easier syntax that could be translated to TT.)

Just my NOK 0,02...
Comment 7 Robin Sheat 2014-05-22 22:43:00 UTC
(In reply to Magnus Enger from comment #6)
> 1. We might not want to do eval on a syspref

I would be hugely wary about letting people do that. It's pretty much opening the system up to them, which is not good for hosted situations.
Comment 8 Robin Sheat 2014-05-26 02:28:33 UTC
Comment on attachment 28416 [details] [review]
Bug 12296 - make search box customisable with OpacCustomSearch

Found some things I missed: making it show up on a few other pages that don't seem to use the masthead, and forgetting the bootstrap theme.
Comment 9 Robin Sheat 2014-05-26 03:31:21 UTC Comment hidden (obsolete)
Comment 10 Marc Véron 2014-06-09 18:24:50 UTC
Does not apply:

Applying: Bug 12296 - make search box customisable with OpacCustomSearch
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Auto-merging installer/data/mysql/sysprefs.sql
Failed to merge in the changes.
Comment 11 Jonathan Druart 2014-06-18 13:48:19 UTC Comment hidden (obsolete)
Comment 12 Owen Leonard 2014-06-18 14:35:28 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2014-06-19 21:09:23 UTC
Hi Owen, please remember to obsolete the original patch :)
Comment 14 Katrin Fischer 2014-06-19 21:29:01 UTC
There are some small things with this patch:
- It includes lots of whitespace changes that would be better in a separate patch.
- It puts the new pref at the end of the alphabetized syspref file in its 
  own SQL statement.
- It includes changes to the prog theme.

I think it would also be nice to include some kind of example code somewhere to give people an idea about how to use the pref and the limitations it has, but that's not a blocker.

Robin, could you please provide a patch with nicer syspref.sql and without prog?
Comment 15 Owen Leonard 2014-06-20 13:04:18 UTC
(In reply to Katrin Fischer from comment #13)

> Hi Owen, please remember to obsolete the original patch :)

Sorry, git-bz has made me lazy by usually doing this for me. Now when it doesn't I'm in trouble.

(In reply to Katrin Fischer from comment #14)

> - It includes lots of whitespace changes that would be better in a separate
> patch.

After doing by best to create the Bootstrap templates with consistent indentation I appreciate that this patch continues it. I think that with options in git for ignoring whitespace during diff and blame it's time to let whitespace changes happen more freely.

Maybe that's a discussion for a dev meeting instead of on this bug.
Comment 16 Robin Sheat 2014-06-22 22:44:03 UTC
(In reply to Katrin Fischer from comment #14)
> There are some small things with this patch:
> - It includes lots of whitespace changes that would be better in a separate
> patch.

All it is is ensuring that the indentation remains consistent. Adding -w to the git commands (e.g. diff and show) will cause the things that are only whitespace changes to be hidden.

> - It puts the new pref at the end of the alphabetized syspref file in its 
>   own SQL statement.
> - It includes changes to the prog theme.

These I'll sort.
Comment 17 Robin Sheat 2014-06-25 02:44:32 UTC Comment hidden (obsolete)
Comment 18 Katrin Fischer 2014-06-28 10:37:31 UTC
Created attachment 29341 [details] [review]
[PASSED QA] Bug 12296 - make search box customisable with OpacCustomSearch

This allows the search box to be replaced by some custom HTML, useful
when you're needing to integrate with some other search system, and
don't want to maintain a template change across upgrades.

Test plan:
* Install patch
* Look at the OPAC, see that nothing has changed.
* Change the OpacCustomSearch syspref to something like <h1>Zuul</h1>
* Look at the OPAC again, you can no longer search, there is only Zuul.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Works as advertised. Seems like it would be really easy to screw up your
OPAC with this feature, but since we already offer other easy ways to
screw up your OPAC I guess this fits in.

New patch changes: removed the bootstrap code, changed the entry in
syspref.sql

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes QA script and tests.
The feature has some limitations, described on the bug.
Comment 19 Tomás Cohen Arazi (tcohen) 2014-10-18 13:39:50 UTC
Created attachment 32520 [details] [review]
Bug 12296: (QA followup) remove rebasing leftover

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Comment 20 Tomás Cohen Arazi (tcohen) 2014-10-18 13:58:30 UTC
Patch pushed to master.

Thanks Robin!