New tab on search result selection

Bug #1028649 reported by Andrew Coomes
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Won't Fix
Wishlist
Unassigned

Bug Description

New Feature:

When a user ctrl-clicks on a search result title a new tab is opened. This allows the user to keep their search page open.

Wiki page: http://www.evergreen-ils.org/dokuwiki/doku.php?id=dev:proposal:new_tab

Blueprint: https://blueprints.launchpad.net/evergreen/+spec/new-tab

Evergreen: 2.4 ( master )
OpenSRF: 2.1
PostgreSQL: 9.1
Linux distribution: Ubuntu 10.04 & 12.04

Feature branch can be found on the working directory at the following location:
user/catalystit/search_result_new_tab

Tags: staffclient ui
Andrew Coomes (acoomes)
tags: added: ui
description: updated
description: updated
description: updated
description: updated
Andrew Coomes (acoomes)
description: updated
Revision history for this message
Kathy Lussier (klussier) wrote :

Hi Andrew,

This sounds useful. I notice you have a pullrequest tag on this, but there is no code. Do you have a git branch for this code so that I can try it out?

Revision history for this message
Andrew Coomes (acoomes) wrote :

Hi Kathy,

Thank you for your interest in this feature! When we posted this there was some confusion regarding the code acceptance process, and we didn't develop our features in separate branches. We are currently in the process of updating our features to newer versions of Evergreen and splitting them into separate branches. Once a feature is ready, we will post it with a link to its git branch. We will also notify the dev mailing list so those interested can look it over and try it out.

description: updated
Andrew Coomes (acoomes)
description: updated
Kathy Lussier (klussier)
tags: removed: pullrequest
Andrew Coomes (acoomes)
description: updated
Andrew Coomes (acoomes)
description: updated
Changed in evergreen:
importance: Undecided → Wishlist
Revision history for this message
Jason Stephenson (jstephenson) wrote :

When you have implemented your feature branch for this bug, please add a reference to the branch in a publicly accessible git repository. Also, if you believe it is ready for testing and inclusion into the public codebase add the pullrequest tag.

Thanks.

Changed in evergreen:
status: New → Incomplete
Changed in evergreen:
status: Incomplete → Triaged
Andrew Coomes (acoomes)
tags: added: pullrequest
Andrew Coomes (acoomes)
description: updated
description: updated
description: updated
Revision history for this message
Jason Stephenson (jstephenson) wrote :

First, a couple of nits to pick:

1. I don't like seeing

+COMMIT;
\ No newline at end of file

and specially

-</div>
+</div>
\ No newline at end of file

in diffs. The latter is particularly egregious, since it indicates that your editor removed a newline that was at the bottom of the file.

2. Please set a name for the author and to go with the signoffs, rather than just "Catalyst IT Services." We like having individuals to blame, err credit, for new code. If multiple people worked on a branch, it is OK to have more than one commit with individual authors. Multiple signoffs are OK, but you'd still need at least 1 core committer to sign off on the code before it goes in.

Now, something that is bigger than a nit. Once I applied your branch on my development server and ran a title search to attempt to test the functionality, I got the following in the Apache2 error log:

[Thu Dec 27 14:54:16 2012] [error] [client 134.241.121.11] egweb: Context Loader error: Can't call method "self_get_pref_lib" on unblessed reference at /usr/local/share/perl/5.14.2/OpenILS/WWW/EGCatLoader/Search.pm line 505.\n, referer: https://jasondev.mvlcstaff.org/eg/opac/login?redirect_to=%2Feg%2Fopac%2Fmyopac%2Fmain

I do not get the error in the absence of your branch.

I believe that there are those who would like a more general Ctrl-click to open anything in a new tab in the staff client.

I know that I would prefer for the changes to NOT affect the patron OPAC at all. Browsers already have builtin methods of opening links in new tabs and windows, and we should not usurp that ability from them.

Revision history for this message
Andrew Coomes (acoomes) wrote :

Jason,

Thank you for your prompt testing and feedback! We appreciate your help and interest in this feature and will address these issues right away.

As it is currently written, this feature should only operate within the staff client and should not affect the native new tab functionality in web browsers. Please let us know if you experience anything to the contrary.

We agree that a global Ctrl-click function would be ideal; however, the additional functionality is beyond our current scope. We expanded on the original feature by adding the Ctrl-click option instead of automatically opening a new tab on click, and there is room for further development here in the future.

Again, thank you for your response. We will notify the mailing list when the requested changes have been made.

Kyle Tomita (tomitakyle)
Changed in evergreen:
assignee: nobody → Kyle Tomita (tomitakyle)
Revision history for this message
Kyle Tomita (tomitakyle) wrote :

Jason,

The code has been fixed to your suggestions and pushed to the same branch. If you have time to review it once more, they would be greatly appreciated.

Thanks,
Kyle

tags: added: staffclient
Revision history for this message
Kyle Tomita (tomitakyle) wrote :
Kyle Tomita (tomitakyle)
Changed in evergreen:
assignee: Kyle Tomita (tomitakyle) → nobody
Revision history for this message
Kathy Lussier (klussier) wrote :

Wow! This one is old. I don't know how much longer this work will be needed with the web client on its way, but, if the functionality is still desired, the branch will need a rebase. Removing pullrequest for now.

tags: removed: pullrequest
Changed in evergreen:
status: Triaged → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.