Bug 3347 - Inconsistencies with tables in opac-shelves.tmpl
Summary: Inconsistencies with tables in opac-shelves.tmpl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: PATCH-Sent (DO NOT USE) minor (vote)
Assignee: Owen Leonard
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-20 02:29 UTC by Garry Collum
Modified: 2012-10-25 23:03 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Comparison of the private and public lists of lists. (46.95 KB, image/jpeg)
2009-06-23 14:53 UTC, Chris Cormack
Details
Proposed change (32.44 KB, patch)
2011-01-05 01:12 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed change, revised (33.32 KB, patch)
2011-01-05 14:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Proposed fix for 3.2.x (32.96 KB, patch)
2011-01-06 18:56 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2010-05-21 01:09:24 UTC


---- Reported by gcollum@gmail.com 2009-06-20 02:29:50 ----

When viewing the Lists in opac-shelves.tmpl, the private list contains a button to add a new list, and the table contains a sort-by column.  Neither one of these is present when viewing the public lists.



---- Additional Comments From oleonard@myacpl.org 2009-06-23 14:23:53 ----

Does this report refer to the list of lists, as opposed to the view which shows the content of an individual list? When I look at the list of lists I don't see what you describe:

Private lists:

1. Log in
2. Click on "My Lists."
3. The list of lists has no sortable columns.
4. There is a "New list" link

Public lists:

1. Log in.
2. Click on "My Lists."
3. Click the "Public Lists" tab.
4. The list of lists has no sortable columns.
5. There is no "New list" link



---- Additional Comments From oleonard@myacpl.org 2009-06-23 14:43:00 ----

Sorry, submitted before I finished my thought. I agree that the "new list" link should display for public and private lists if the user is logged in. I'm trying to decide if the "sort by" column is appropriate to display given that the display of public lists is going to show lists by other users...I want to avoid showing information to the general public that they're not interested in seeing.



---- Additional Comments From gcollum@gmail.com 2009-06-23 14:53:34 ----

Created an attachment
Comparison of the private and public lists of lists.

The patch that I have submitted adds a "sort by" and an "add new" button to the public list view.



---- Additional Comments From gcollum@gmail.com 2009-06-23 14:58:03 ----

No problem.  I think the Librarian in me has a tendency to show more information than people want.  I could see that the 'sort by,' maybe we should call it 'sorted by,' column being helpful to viewers if the list is lengthy.

As I look at the lists again, I wonder if the 'type' column is necessary.  Is it redundant?



---- Additional Comments From oleonard@myacpl.org 2009-06-23 16:38:47 ----

Thanks for the patch Gary. Please be sure to attach your patch to the bug report as well as sending it to the patches list. I hadn't caught up with the patches list traffic so I didn't know you'd already submitted something. You can also assign bugs to yourself if you know you're going to submit a patch. All of that helps others (like me!) avoid working on a bug for which a patch is in progress.

I like your suggestion to change the column name to "sorted by."

Regarding the display of list "type," the lists which display under the "Public lists" tab could be either Public or Open, so there is an important distinction to make. What I'm realizing now is that the "Your Lists" tab is deceptive because it's really "Your Private Lists" only. It's confusing that the user won't see their own public or open lists under that tab.



---- Additional Comments From gcollum@gmail.com 2009-06-23 16:51:05 ----

Thanks Owen.  Will do in the future.

Would you like me to send another patch tonight reversing the addition of the 'sort by' column in the public tab, or should we keep it and just rename it?  That's almost trivial compared to the "New List" button.  I really like that appearing on both tabs.



---- Additional Comments From oleonard@myacpl.org 2009-06-23 17:38:54 ----

Thinking about it some more, I realize that for JavaScript-enabled users we're bypassing the designated sortfield with the jquery tablesorter plugin. That means *all* lists are sorted by title by default. So the "sort by" information is potentially incorrect for those users. I'd love to be able to switch the behavior of the tablesorter based on the sortfield designation, but it doesn't look like the "viewshelf" template receives that bit of data from the database.



---- Additional Comments From gcollum@gmail.com 2009-06-23 18:04:14 ----

Yep.  I was already looking into creating some sorting variables to pass from perl to incorporate into jquery.

Also I'm not getting proper sorting of titles in firefox with the jquery tablesorter, it works in IE.  But I haven't figured out why it doesn't work in firefox.  It has something to do the the "article" subroutine.  I haven't created a bug report, because I haven't experimented enough to figure out if it is just my firefox, or all firefoxes.  

I would also like to add tablesorter sorting to the "list of lists" tables that we are talking about.



---- Additional Comments From gmcharlt@gmail.com 2009-06-29 19:08:22 ----

Pushed first set of patches.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:09 UTC  ---

This bug was previously known as _bug_ 3347 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=3347
Imported an attachment (id=1228)

Actual time not defined. Setting to 0.0
The original submitter of attachment 1228 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.

Comment 1 Owen Leonard 2011-01-05 01:12:12 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2011-01-05 12:47:49 UTC
Tried on the new patch submitted yesterday and it did not apply.



Applying Fix for Bug 3347 - Inconsistencies with tables in opac-shelves.tmpl
.dotest/patch:120: trailing whitespace.
#toolbar { 
error: patch failed: koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tmpl:6
error: koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tmpl: patch does not apply
Using index info to reconstruct a base tree...
<stdin>:120: trailing whitespace.
#toolbar { 
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merged C4/VirtualShelves.pm
Auto-merged koha-tmpl/opac-tmpl/prog/en/css/opac.css
Auto-merged koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tmpl
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/modules/opac-shelves.tmpl
Failed to merge in the changes.
Patch failed at 0001.
When you have resolved this problem run "git-am -i -3 --resolved".
If you would prefer to skip this patch, instead run "git-am -i -3 --skip".
nengard@debian:~/kohaclone$
Comment 3 Owen Leonard 2011-01-05 14:52:37 UTC
Created attachment 2945 [details] [review]
Proposed change, revised
Comment 4 Chris Cormack 2011-01-05 20:55:31 UTC
Pushed, please test
Comment 5 Owen Leonard 2011-01-06 18:56:36 UTC
Created attachment 2960 [details] [review]
Proposed fix for 3.2.x
Comment 6 MJ Ray (software.coop) 2011-02-01 09:19:54 UTC
This bug is mentioned in:
Fix for Bug 3347 - Inconsistencies with	tables in opac-shelves.tmpl http://lists.koha-community.org/pipermail/koha-patches/2011-January/013508.html
Fix for Bug 3347 -	Inconsistencies with tables in opac-shelves.tmpl http://lists.koha-community.org/pipermail/koha-patches/2011-January/013516.html
Fix for Bug 3347 -	Inconsistencies with tables in opac-shelves.tmpl http://lists.koha-community.org/pipermail/koha-patches/2011-January/013517.html
Fix for Bug 3347 - Inconsistencies with tables in opac-shelves.tmpl http://lists.koha-community.org/pipermail/koha-patches/2011-January/013533.html
Fix for Bug 3347 - Inconsistencies	with tables in opac-shelves.tmpl http://lists.koha-community.org/pipermail/koha-patches/2011-January/013511.html
Fix for Bug 3347 - Inconsistencies	with tables in opac-shelves.tmpl http://lists.koha-community.org/pipermail/koha-patches/2011-January/013542.html