Patch attached adds in [-] in the advanced search page before the [+] to allow to remove search terms. To test click on the More options link in the advanced search page. Patch sponsored by the CCSR ( http://www.ccsr.qc.ca ).
Created attachment 11454 [details] [review] patch to add link to remove a search term
Don't forget to assign the bug to yourself when you submit a patch, and to mark it as "needs signoff."
I'll get my petty comment out of the way first: Shouldn't it be [+][-] instead of [-][+]? I don't know why I think that. More importantly: I think the [-] option should only be applied to fields which are dynamically added. If you remove a field which wasn't dynamically added you're using the operation as a defacto "clear" button, which we offer only in rare circumstances. The other problem with being able to remove a "hard-coded" row with [-] is that it makes it possible to remove the last row which contains the [+]. If you remove that one you don't have the option anymore to add more rows. The final version of this enhancement should prevent/work around that problem.
(In reply to comment #3) > I'll get my petty comment out of the way first: Shouldn't it be [+][-] > instead of [-][+]? I don't know why I think that. > > More importantly: I think the [-] option should only be applied to fields > which are dynamically added. If you remove a field which wasn't dynamically > added you're using the operation as a defacto "clear" button, which we offer > only in rare circumstances. > > The other problem with being able to remove a "hard-coded" row with [-] is > that it makes it possible to remove the last row which contains the [+]. If > you remove that one you don't have the option anymore to add more rows. The > final version of this enhancement should prevent/work around that problem. I disagree with the +- order because I feel like the - should be aligned. But it doesn't matter. I disagree about having the - only on dynamic lines. I do like it being some kind of clear. If I type words in 3 fields and change my mind on one I can remove it and add a new one. I have a few possible solutions and ideas and I would like your opinion on the best one to take. What I propose is to have a link "Add a new search term" at the top instead of the little [+]. That way it would always be in the same position and it would be clearer. In most app the + is to show something hidden, so a text link would help imo. Also I will change my patch to have a small red x instead of a - for the same reasons. Would that be alright with you ?
I think I like the +- buttons because they are very fast and flexible. I can add a search field whereever I want. I don't have to search for a button on top but can do so right at the end of the field I last typed in.
This is a nice enhancement I would like to see in the Opac. I agree with Katrin, liking +- buttons as well. Adding / removing options works well. However I propose to change the wording of "More options"/"Fewer options" Reason: If you remove all options with [-] and have only one left, "Fewer options" is kind of nonsense, because it means to restore the original (3) options, i.e. more, not fewer. Better wording would be something like "Manage options" (instead of "More optons") and "Default options" (instead of "Fewer options") If someone removes the last line with [+] , he/she can "recover" by switching back to "Default options". ("Fewer opions" would be misleading.) Marc
Hi Marc, Hmm, I am not sure it works like this. You only see the +/- when you have the screen with "more" options. So if you accidently delete all your search fields and are left with only one, the fewer options will bring you back to 3 search fields, but you won't have +/- or the boolean pull downs available - so the screen will change and it will not be the same search form you have seen before. That will require switching back and forth. I think maybe the javascript needs to be changed to not allow deleting of the lsecond line or allow cloning of the first with adding boolean search options on this step. I know it's much harder to do, but I don't feel comfortable with the feature as it is now :(
Created attachment 12399 [details] [review] Bug 8600 - Remove search terms in advanced search Enhanced version with better handling of [+] and [-] buttons. [+] adds new fields aftert current line [-] removes current line [-] is not visible if only one line left Marc
Created attachment 12402 [details] [review] Bug 8600 - Remove search terms in advanced search Enhanced version with better handling of [+] and [-] buttons. [+] adds new fields aftert current line [-] removes current line [-] is not visible if only one line left Marc Rebased...
Created attachment 12409 [details] [review] Bug 8600 - Remove search terms in advanced search Enhanced version with better handling of [+] and [-] buttons. [+] adds new fields aftert current line [-] removes current line [-] is not visible if only one line left Marc Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
I like the feature! Two things: - I would prefer the newly added fields to be empty, at the moment the contents (if any) of the field where you click [+] is duplicated. That does not make sense since you would most likely not want that in the duplicated field - Also I would like a more fancy look of the buttons at some point :) Anyway works for me as it is, signed off.
More precisely, - it would make sense to me to duplicate the state of operator and search index of the row where I hit [+], but that does not happen, it will always be "and" and "Keyword" - it does not make much sense to me to clone the text field, which is done
Mirko, The + button behaves as it did before (regarding the value and preserving the operator etc.). It simply makes a clone of the row. My aim was to get the [-] button working for having it in the next release and defer the + to a follow up. Marc
Changing target version so this bug report will show up in the list of 3.12-targeted features and doesn't get lost in the shuffle during feature freeze.
QA Comment: It works as expected. But I don't know if <a href="javascript:;'> is the best way to do that. In Koha, we use <a href="#" onclick="function(); return false;"> But in an accessibility point of view, it is not good. Owen, could you give your opinion on it please ?
(In reply to comment #15) > But I don't know if > <a href="javascript:;'> > is the best way to do that. > In Koha, we use > <a href="#" onclick="function(); return false;"> Quite right. Even better, add an id or class to the link and move the onclick handling to the $(document).ready() block. In my opinion this requires a correction.
(In reply to comment #16) > Quite right. Even better, add an id or class to the link and move the > onclick handling to the $(document).ready() block. In my opinion this > requires a correction. Thanks Owen. So I switch to failed qa
Created attachment 16560 [details] [review] Rewrote part of the patch based on previous feedback. Replaced the href and onclick by good old jQuery code and clear the value of the cloned input.
Patch tested with a sandbox, by Cedric Vita <cedric.vita@dracenie.com>
Created attachment 16592 [details] [review] Bug 8600 - Remove search terms in advanced search Rewrote parts of previous patch by Marc Veron to remove remove href and onclick of links and instead use jQuery Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>
Created attachment 18649 [details] [review] Bug 8600 - Remove search terms in advanced search Rewrote parts of previous patch by Marc Veron to remove remove href and onclick of links and instead use jQuery Signed-off-by: Cedric Vita <cedric.vita@dracenie.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Pushed to master. Thanks, Frédérick! A couple notes: - I reworded the commit description to be clearer - I pushed a follow-up patch to remove the unneeded debug logging to the JavaScript console.
(In reply to comment #22) > - I pushed a follow-up patch to remove the unneeded debug logging to the > JavaScript console. The console.log should be fail the qa tests. Chris, is your local repo up-to-date?
This patch has been pushed to 3.12.x, will be in 3.12.1. Thanks Frédérick!