Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover
Summary: In advanced search(more), [+] and [-] should behave like links on hover
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Owen Leonard
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-11 12:56 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2014-12-07 20:02 UTC (History)
3 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover (2.16 KB, patch)
2013-06-11 13:02 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 10444 - (CCSR followup) In advanced search(more), [+] and [-] should behave like links on hover (972 bytes, patch)
2013-06-11 14:51 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover (2.26 KB, patch)
2013-06-11 15:16 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10444 - (CCSR followup) In advanced search(more), [+] and [-] should behave like links on hover (1.05 KB, patch)
2013-06-11 15:17 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover (2.32 KB, patch)
2013-06-12 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10444 - (CCSR followup) In advanced search(more), [+] and [-] should behave like links on hover (1023 bytes, patch)
2013-06-12 14:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10444 - (CCSR followup) In advanced search(more), [+] and [-] should behave like links on hover (1.06 KB, patch)
2013-06-12 14:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover (2.37 KB, patch)
2013-06-12 15:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover (2.21 KB, patch)
2013-06-13 09:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10444: Fix prevent the default action on click (1.33 KB, patch)
2013-06-13 09:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10444 - (CCSR followup) In advanced search(more), [+] and [-] should behave like links on hover (1.13 KB, patch)
2013-06-14 12:07 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover (2.26 KB, patch)
2013-06-14 12:07 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 10444: Fix prevent the default action on click (1.38 KB, patch)
2013-06-14 12:07 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover (2.33 KB, patch)
2013-06-14 12:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10444 - (CCSR followup) In advanced search(more), [+] and [-] should behave like links on hover (1.11 KB, patch)
2013-06-14 12:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10444: Fix prevent the default action on click (1.44 KB, patch)
2013-06-14 12:17 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2013-06-11 12:56:58 UTC

    
Comment 1 Tomás Cohen Arazi (tcohen) 2013-06-11 13:02:13 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi (tcohen) 2013-06-11 14:51:31 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2013-06-11 15:16:08 UTC Comment hidden (obsolete)
Comment 4 Owen Leonard 2013-06-11 15:17:35 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2013-06-12 14:24:43 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2013-06-12 14:24:46 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2013-06-12 14:25:14 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2013-06-12 14:27:15 UTC
QA comment:
Works as expected except for the click reset the scroll at the top. I amended the patch in order to add a onclick and to trigger the event.
Marked as Passed QA.
Comment 9 Galen Charlton 2013-06-12 14:55:01 UTC
(In reply to comment #8)
> QA comment:
> Works as expected except for the click reset the scroll at the top. I
> amended the patch in order to add a onclick and to trigger the event.
> Marked as Passed QA.

I'm not quite sure that your amendment was attached to this bug -- please double-check.
Comment 10 Jonathan Druart 2013-06-12 15:03:44 UTC Comment hidden (obsolete)
Comment 11 Galen Charlton 2013-06-12 18:28:08 UTC
I disagree with how the QA modification was implemented.  Rather than adding onclick attributes, the existing jQuery click handlers should be modified to return false.  Doing it that way is more consistent with our current push for unobtrusive Javascript.
Comment 12 Jonathan Druart 2013-06-13 09:08:42 UTC
(In reply to comment #11)
> I disagree with how the QA modification was implemented.  Rather than adding
> onclick attributes, the existing jQuery click handlers should be modified to
> return false.  Doing it that way is more consistent with our current push
> for unobtrusive Javascript.

Yes of course, but nodes are generated after the load of the page. So I thought it would be difficult. But in fact no, I will resubmit patches.
Comment 13 Jonathan Druart 2013-06-13 09:12:55 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2013-06-13 09:13:00 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall (khall) 2013-06-14 11:55:55 UTC
I'm afraid the link underlines still shows for the ccsr theme. Please add a followup to modify the ccsr css as well.
Comment 16 Jonathan Druart 2013-06-14 12:01:58 UTC
(In reply to comment #15)
> I'm afraid the link underlines still shows for the ccsr theme. Please add a
> followup to modify the ccsr css as well.

My fault, I obsoleted it.
Comment 17 Kyle M Hall (khall) 2013-06-14 12:07:03 UTC Comment hidden (obsolete)
Comment 18 Kyle M Hall (khall) 2013-06-14 12:07:11 UTC Comment hidden (obsolete)
Comment 19 Kyle M Hall (khall) 2013-06-14 12:07:24 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2013-06-14 12:14:03 UTC
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

So, marked as Passed QA :)
Comment 21 Jonathan Druart 2013-06-14 12:17:10 UTC
Created attachment 18997 [details] [review]
Bug 10444 - In advanced search(more), [+] and [-] should behave like links on hover

This patch add href="#" to the [+] and [-] links for adding and removing
search terms. It also defines its style so it doesn't show underlined.

To test:
- Browse your OPAC to "Advanced Search" and then "More options".
- The [+] and [-] don't change the cursor on hover
- Apply the patch, reload and see the cursor changes
- Functionality should remain unchanged

Regards
To+

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 22 Jonathan Druart 2013-06-14 12:17:14 UTC
Created attachment 18998 [details] [review]
Bug 10444 - (CCSR followup) In advanced search(more), [+] and [-] should behave like links on hover

Same fix for CCSR.

Regards
To+

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 23 Jonathan Druart 2013-06-14 12:17:17 UTC
Created attachment 18999 [details] [review]
Bug 10444: Fix prevent the default action on click

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 24 Galen Charlton 2013-06-14 15:12:33 UTC
Pushed to master.  Thanks, Tomás and Jonathan!
Comment 25 Tomás Cohen Arazi (tcohen) 2013-06-28 15:16:38 UTC
This patch has been pushed to 3.12.x, will be in 3.12.2.

Thanks everyone!