Created attachment 22349 [details] [review] Bug 11127: OPAC suggestion: the delete link is active when no suggestion selected To reproduce: 1/ go on opac-suggestions.pl 2/ create a suggestion 3/ click on the delete link without checking any checkbox The form is submitted with a message "The selected suggestions have been deleted.". Test plan: 1/ after applying the patch, click on delete without any checkbox checked, a js alert should occur. 2/ check at least a cb and click on the delete link. The deletion should work.
Should I produce the same fix for others themes (bootstrap and ccsr)?
Instead of adding the "disabled" attribute to an element which isn't really supposed to have it (according to the specification) I think it would be better to check for the existence of a checked checkbox: var checkedBoxes = $("input:checked"); if ($(checkedBoxes).size() == 0) { alert(MSG_NO_RECORD_SELECTED); return false; }
Jonathan: If there are still suggestions, there is also the Delete selected button below. If you click on that button without a selection, it still says: Selected sugg have been deleted. For consistency, I would recommend adjusting that too.
Owen, Why are there 2 ways to delete selected suggestions? Should not we keep only the button on the bottom?
ping Owen
(In reply to Jonathan Druart from comment #6) > ping Owen Sorry, here I am! (In reply to Jonathan Druart from comment #5) > Why are there 2 ways to delete selected suggestions? Should not we keep only > the button on the bottom? I don't see any reason not to offer both options. We do the same on the lists page and the on the user account page with renewal options.
Created attachment 25818 [details] [review] Bug 11127: OPAC suggestion: the delete link is active when no suggestion selected To reproduce: 1/ go on opac-suggestions.pl 2/ create a suggestion 3/ click on the delete link without checking any checkbox The form is submitted with a message "The selected suggestions have been deleted.". Test plan: 0/ enable the bootstrap theme 1/ after applying the patch, click on delete without any checkbox checked, a js alert should occur. 2/ check at least a cb and click on the delete link. The deletion should work.
Created attachment 25823 [details] [review] [Signed-off] Bug 11127: OPAC suggestion: the delete link is active when no suggestion selected To reproduce: 1/ go on opac-suggestions.pl 2/ create a suggestion 3/ click on the delete link without checking any checkbox The form is submitted with a message "The selected suggestions have been deleted.". Test plan: 0/ enable the bootstrap theme 1/ after applying the patch, click on delete without any checkbox checked, a js alert should occur. 2/ check at least a cb and click on the delete link. The deletion should work. Followed test plan. Patch behaves as expected. Signed-off-by: Marc Véron <veron@veron.ch>
Hi Jonathan, I like the idea and it works nicely :) The only thing I am a bit concerned about is reusing the message from the cart. I think right now they are always in the template even if the cart is turned off, but that might change at some point. Also the message might read a bit nicer as "No suggestion has been selected" - as item is also a bit confusing for translations.
Created attachment 26073 [details] [review] Bug 11127: Define a specific message for suggestion
Hi Jonathan, the alert doesn't come up for me now and I can't see the new message in the template. Maybe we could just add the string directly into the alert? There seems to be an example for this a bit below. I think the MSG.* mostly exist because the Javascript for those is in a separate .js file.
The alert isn't coming up now because the MSG is defined in the prog include, but the template is only modified for the bootstrap theme. I would probably just put the MSG definition in the template itself since it's only relevant to that one page.
Created attachment 26120 [details] [review] Bug 11127: Define a specific message for suggestion
(In reply to Owen Leonard from comment #13) > The alert isn't coming up now because the MSG is defined in the prog > include, but the template is only modified for the bootstrap theme. Arg stupid mistake, sorry Katrin!
Hi Jonathan, I am sorry, but after takig a look at the code, I think we need to move the line with the message out of the IF statement, or it will depend on cart and lists being turned on: [% IF Koha.Preference( 'opacbookbag' ) == 1 or Koha.Preference( 'virtualshelves' ) == 1 %]
Created attachment 26168 [details] [review] Bug 11127: Define a specific message for suggestion
Created attachment 26307 [details] [review] [SIGNED OFF] Bug 11127: Define a specific message for suggestion Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 26308 [details] [review] [PASSED QA] Bug 11127: OPAC suggestion: the delete link is active when no suggestion selected To reproduce: 1/ go on opac-suggestions.pl 2/ create a suggestion 3/ click on the delete link without checking any checkbox The form is submitted with a message "The selected suggestions have been deleted.". Test plan: 0/ enable the bootstrap theme 1/ after applying the patch, click on delete without any checkbox checked, a js alert should occur. 2/ check at least a cb and click on the delete link. The deletion should work. Followed test plan. Patch behaves as expected. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script, works as described.
Created attachment 26309 [details] [review] [PASSED QA] Bug 11127: Define a specific message for suggestion Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Pushed to master. Thanks, Jonathan!
Pushed to 3.14.x, will be in 3.14.08
Oups, the title of this bug was the one of Bug 11934 : "Replace given by if-else statements"
Sorry, I've add a odd behavior with my Lazarus Plugin. I restore this bug from history.